Skip to content

feat: common test suite generation APIC-186 #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Dec 2, 2021
Merged

feat: common test suite generation APIC-186 #22

merged 16 commits into from
Dec 2, 2021

Conversation

millotp
Copy link
Collaborator

@millotp millotp commented Nov 30, 2021

🧭 What and Why

🎟 JIRA Ticket: APIC-186

Generation of the CTS for the js client, using jest.
This is a first draft, it won't work for other languages right away.

Changes included:

  • Created search.json, the first CTS spec !
  • generateCTS.ts is a script that concat all the CTS spec, and generate the code according to the mustache templates

🧪 Test

  • yarn cts:generate
  • yarn cts:test

@millotp millotp self-assigned this Nov 30, 2021
Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

general review just for fun, did not checked the JS yet

doc/CTS.md Outdated
## How to add a new langage

- Create a template in `test/CTS/templates/<your langage>.mustache` that parse a array of test into your test framework of choice
- Add the langage in the array `langages` in `tests/generateCTS.ts`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could define all the language in the array and only generate tests if the folder exists, so it requires less manual work later

@millotp millotp marked this pull request as ready for review December 1, 2021 14:37
@millotp millotp requested review from shortcuts and damcou December 2, 2021 12:09
Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a few test and it seems to work as expected! GG

My comments are mostly implementation details, I think making the script as clean/simple as possible will be benefit for us in the future

Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems good to go for me, do we want to maybe add a new CI workflow to test the generation there?

(and GG, of course :D)

Copy link
Contributor

@damcou damcou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for not being very active here, this looks really good, looking forward to see it work on a larger scope ❤️

@millotp
Copy link
Collaborator Author

millotp commented Dec 2, 2021

This seems good to go for me, do we want to maybe add a new CI workflow to test the generation there?

(and GG, of course :D)

Good idea, I'll do it in another PR.

@millotp millotp merged commit f2a2670 into main Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants