check if we can have one `integration.test.js` with snapshots for all websocket client under `websocket/test` with as less code duplication as possible

[derberg]

if you look into python/test/integration.test.js you see that once I introduced listFiles, now basically all tests are the same, and only input arguments are different. I believe we can create a helper for generating snapshots that basically accepts different templateParams and fixtures and testsResults. There is too much boilerplate code. Also investigate, maybe we should have integration tests only in websocket/test and not in separate language-based folders

[derberg]

if we abstract it properly, then our test could look like so in this PR https://github.com/asyncapi/community/pull/1837 where you just provide different test cases by providing different generator options

[derberg]

@ItshMoh will work on it

[derberg]

some details in https://github.com/asyncapi/generator/issues/1505 already

[ItshMoh]

I will be working on this in this week.

[ItshMoh]

While exploring the Issue the option that i found. The first option in which we can have the integration-test-helpers.js in side the packages/templates/clients/websocket/test folder. There will be still a lot of code in the respective integration.test.js file in each language. I tried to make a helper currently only handling the javascript language websocket. It passes all the test. Here is the commit link.

In the second option as mentioned in this PR where we can have the whole integration.test.js in the websocket/test/integration.test.js. Some of the main issues in this option is discussed here by @Adi-204. Also When testcases will increase the testcase list will also increase and the function which will be generating the file and matching the snapshot will have to be handling more cases( a lot of if and else). a list of testcases will look something like

const testCases = [
  {
    name: 'simple client for postman echo',
    fixture: commonFixtures.postman,
    params: (templateConfig) => ({ 
      server: 'echoServer', 
      [templateConfig.defaultClientFileNameParam]: templateConfig.mainOutputFile || templateConfig.defaultClientFileName,
      appendClientSuffix: true 
    }),
    outputSubDir: (templateName) => `${templateName.toLowerCase().replace(/\s+/g, '_')}_postman`
  },
  {
    name: 'simple client for hoppscotch echo',
    fixture: commonFixtures.hoppscotch,
    params: (templateConfig) => ({ 
      server: 'echoServer',
      [templateConfig.defaultClientFileNameParam]: templateConfig.mainOutputFile || templateConfig.defaultClientFileName,
    }),
    outputSubDir: (templateName) => `${templateName.toLowerCase().replace(/\s+/g, '_')}_hoppscotch`
  },
  // ... more common test cases
  // Error case:
  {
    name: 'missing server param for hoppscotch (JS specific check, or make helper handle it)',
    fixture: commonFixtures.hoppscotch,
    params: (templateConfig) => ({ 
      [templateConfig.defaultClientFileNameParam]: 'client.js'
    }),
    outputSubDir: (templateName) => `${templateName.toLowerCase().replace(/\s+/g, '_')}_hoppscotch_error`,
    expectedError: 'This template requires the following missing params: server',
    // Optionally, restrict this test case to specific templates
    // appliesToTemplates: ['Javascript Websocket Client'] 
  }
]

The third option is where we can have a common-integration-test.js file in the websocket/test folder for the test that are almost identical for all languages. And some language specific tests for handling the language specific cases like the missing server param clientFileName param in js. These test will be in the specific language folder. It may give us more control in future when there will more testcases in the future.

@derberg @Adi-204 What are your opinion?

[ItshMoh]

I am also not sure about the option 3 regarding How the temp folder in which the client are generated will function? What will be temp folder location.

[Adi-204]

@ItshMoh I agree with your take on both approach 1 and 2. can you please mention a bit more detail about approach 3. I think we can keep location of temp inside test folder only because if we removed integration.test.js from all the templates still component testing will be done in test folder only for now atleast maybe change that also in future.

[derberg]

yeah, I'm also interested in more details on approach 3

[ItshMoh]

Hey sorry for late reply, I will be giving a short implementation of option 3 in 4 days.

[ItshMoh]

Hey @derberg sorry I would not be able to continue with the research due to other time commitment.

[Adi-204]

@derberg I have directly create a PR for this issue to show one of the approach. If it is not good we will close it.