Skip to content
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

Improving GitHub Actions tests #886

Merged
merged 14 commits into from
Aug 22, 2022

Conversation

israelboudoux
Copy link
Contributor

This PR adds missing tests to Github Actions pipeline:

  • Tests under common-files are now part of the pipeline;
  • Missing tests under /test like kem-dem.test.mjs and timber.test.mjs were added too;
  • A new test RogerTaule related to circuits was created was included in the pipeline.

@israelboudoux israelboudoux marked this pull request as ready for review August 19, 2022 18:33
Copy link
Contributor

@imagobea imagobea left a comment

Choose a reason for hiding this comment

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

Hi @israelboudoux, nice contribution towards a more organised code base!! 💪

I'm just going to suggest a few more changes which you can apply if you agree. It's about script commands on the package.json (below).

I think that might be worth to rename as well some of the jobs on the GitHub workflows. Eg use always test in front of jobs that are actually running tests.

Last but not least, is it possible that we are not running the rollback-resync test?! 🤔

Cheers!!

"test":
"test-e2e-protocol"
"test-gas" >> "test-e2e-gas" 
"test-circuits" >> "test-e2e-circuits" 
"test-e2e-tokens"
"test-erc20-tokens" >> "test-e2e-tokens-erc20" OR "test-tokens-erc20"
"test-erc721-tokens" (like erc20)
"test-erc1155-tokens" (like erc20)
"test-erc20-cli" >> "test-client-erc20" - The word `cli` typically refers to old sdk (cli/lib) 
"test-adversary"
"test-all-adversary" >> "test-adversary-all"
"test-general-stuff" >> "test-others" (?)

@imagobea imagobea added the One more approval needed One reviewer has approved this PR but another is needed label Aug 22, 2022
Copy link
Contributor

@daveroga daveroga left a comment

Choose a reason for hiding this comment

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

LGTM

@israelboudoux israelboudoux merged commit a18899d into master Aug 22, 2022
@israelboudoux israelboudoux deleted the israelboudoux/github-actions-tests#2 branch September 30, 2022 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
One more approval needed One reviewer has approved this PR but another is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants