Restructure the test schema files for better maintainablility

[jonaslagoni]

Reason/Context

When testing in the parser we use a combination of both inline strings and files where I find it confusing in the way it is structured. Often it is very specific changes we want to look for and the way the files and inline constants are structured are not very intuitive. I see this as a pre-step to adding more tests for our iterators and anonymous namings.

Description

I would suggest we structure the files the following way where <Files for test x> is the test file name such as parse_test.js test -- files -- -- <Files for test x> -- -- -- input -- -- -- output

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity :sleeping: It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation. Thank you for your contributions :heart:

[KhudaDad414]

I agree. Last day I was adding some test and I was thinking of the same thing. I am not sure about your proposed structure but one thing is for sure: the tests are not well structured.

[jonaslagoni]

@KhudaDad414 Agreed my structure might be far from ideal 😅 Do you have any suggestions on how we can do it?

[KhudaDad414]

@jonaslagoni I was searching through other libraries to find a better solution. sorry to disappoint but I wasn't able to find a better structure. I can open a PR making the changes.

[KhudaDad414]

@jonaslagoni should I go for it?

[jonaslagoni]

Awesome @KhudaDad414, if you have a vision on how we can structure it differently than we are now which provides better maintainability go for it 👍

[KhudaDad414]

@jonaslagoni what do you think about this:

[KhudaDad414]

We move all of the folders containing test files into a new folder called files. Then create a new folder for outputs, extract all of the outputs from variables into the files inside outputs folder?

[KhudaDad414]

My reasoning:

1. most of the files belong to parse tests. If we categorise them by test, then nearly all of the files will go into the parse-test-files folder.

2. some files are used by more than one test, like good/asyncapi.yaml. what should we do about them?

3. are we going to mix good and bad files? If not then for each input folder of each test we will have two more folders for good and wrong. That's too much folders😀

[derberg]

hello, I'm a party pooper 😆 and want to challenge you with a question 😄

is it possible, that we simply get rid of all those test files and instead use test files from TCK only?

[jonaslagoni]

@derberg beat me to it 😄 Problem is TCK is not set up to be integrated (as far as I know).. It would indeed remove some of the files, but some are used to ensure correct output as well, such as anonymous messages and schemas are given ids... Not sure exactly how we should solve this. It is some serious refactoring of the entire testing setup we are talking about then.

[derberg]

Problem is TCK is not set up to be integrated

then we should finally do it 😄 just need to publish test files to npm as we do with the spec maybe? -> https://github.com/asyncapi/asyncapi-node

such as anonymous messages and schemas are given ids

but these are not in files but inside test in a variable as expected output right?

It is some serious refactoring of the entire testing setup we are talking about then.

yeah 😓 I'm just not sure simple restructure will actually help. And we anyway run tck against parser later, why not do it here on a PR level. @KhudaDad414 rings the bell? similar to the other issue

---

but as I said, I'm a party pooper 😄

[KhudaDad414]

@derberg Now that's some serious solution to this issue.😀

[derberg]

[jonaslagoni]

then we should finally do it 😄 just need to publish test files to npm as we do with the spec maybe? -> https://github.com/asyncapi/asyncapi-node

Maybe yea... Or provide iterators that can be used directly in tests so you would not have to create this in each library. Would be great to use TCK for templates as well (of course only the valid files would be useful there, maybe?). So we have to think beyond just a specific library.

but these are not in files but inside test in a variable as expected output right?

No, we have some direct tests in the parser test which check this against a provided file. Of course, at the moment it is checked using a variable, bus as @KhudaDad414 suggests we move this to an expected output file instead. Having such as large JSON object in a variable does not make it easy to see changes. - https://github.com/asyncapi/parser-js/blob/2b1d06fef5600caf4893638c58b568fe8d71ced4/test/parse_test.js#L30

yeah 😓 I'm just not sure simple restructure will actually help. And we anyway run tck against parser later, why not do it here on a PR level. @KhudaDad414 rings the bell? similar to the other issue

Would be great to integrate TCK into the PR CI workflow, not sure it is as easy as it sounds. Depends on the library it is used in.

[KhudaDad414]

I'm going to experiment with some solutions to integrate tck into the parser.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity :sleeping: It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation. Thank you for your contributions :heart:

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity :sleeping:

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience :heart:

[derberg]

Hey, folks, is this one still valid? or do we agree better invest in TCK?

[KhudaDad414]

@derberg I think we should go with TCK. If I'm remembering correctly I opened an issue on this, will come back and try to remember what needs to be done 😄

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity :sleeping:

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience :heart:

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity :sleeping:

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience :heart:

[magicmatatjahu]

Can we close this issue? In the next-major branch, the tests structure looks like https://github.com/asyncapi/parser-js/tree/next-major/test so maybe this fit to the better solution?

cc @jonaslagoni @derberg @KhudaDad414

[jonaslagoni]

Sounds reasonable yes 👍

[KhudaDad414]

Sounds good.