Generator: Create tests for the different render engines

[jonaslagoni]

Reason/Context

At the moment we do not have a single test ensuring that the rendering works as expected. We need this to ensure consistency in PR's so we know it works as expected and does not introduce unintended side effects.

[derberg]

something more than that the integration test that I introduce? I mean the one that uses both render engines and then validates the result against the snapshot?

[jonaslagoni]

I would say so yes @derberg, when changes are introduced such as https://github.com/asyncapi/generator/pull/518 we have no tests ensuring the expectation of the functions.

[derberg]

@jonaslagoni maybe we should just not allow features without tests. I for example could not imagine changing installation without providing tests. I actually added tests first to be able to work on changing the code responsible for installation. To many of us started working on the project and it will be easier to break things now. So I would vote, ship slower but with tests

[jonaslagoni]

I agree, similar to our shape-up process they have to include tests, but this one slipped through from my side. @magicmatatjahu @smoya @fmvilas thoughts?

[fmvilas]

Yes, before we used to be Lukasz and me only and had to rush a lot but now we can just slow down and ship better quality software. That includes tests, of course.

[smoya]

I agree folks @derberg and @fmvilas here.
In addition, I think we should encourage adding tests not only for the feature you write but for the surroundings. As stated in the Clean Code book, "Always leave the campground cleaner than you found it".

[magicmatatjahu]

Simple integration tests are added in this PR: https://github.com/asyncapi/generator/commit/2f64f1bf01224497dfe256d309e7e9ae9a0290bf
Unit tests (based on mocks) for React will be added in this PR https://github.com/asyncapi/generator/pull/518

[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:

[derberg]

@jonaslagoni is this one still valid?

[jonaslagoni]

We still don't have any test ensuring nunjucks rendering works. So I would say it is still relevant

[derberg]

we have integration test that generates using html template https://github.com/asyncapi/generator/blob/master/test/integration.test.js which is nunjucks

[jonaslagoni]

While it does test a template with nunjucks it does not explicitly test the nunjucks rendering. If the template switches at some point the generator no longer test that functionality.

[derberg]

While it does test a template with nunjucks it does not explicitly test the nunjucks rendering

what do you mean? what would explicitly test the nunjucks rendering test do? We know what is the future of Nunjucks, so I just want to be sure we are not going to invest to much time in testing something that has no future here 😄

If the template switches at some point the generator no longer test that functionality.

test is hardcoded to explicit template version, nunjucks one, it doesn't follow next releases as it is designed in the way that it tests output of testing version X to make sure it doesn't contain feature added in version Y

[jonaslagoni]

Ahh, didn't see the 0.18.0 part. Guess we are indirectly testing it, so lets just close the issue :)