-
Notifications
You must be signed in to change notification settings - Fork 370
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
Specify topology for n-ary chain tests #4039
Conversation
…in order to use a specified topology
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing! Left some nitpicks inline already, feel free to disregard if you had other plans in mind.
Co-authored-by: Romain Ruetschi <romain@informal.systems> Signed-off-by: Luca Joss <43531661+ljoss17@users.noreply.github.com>
Excellent work! Any insights on how much time this buys us on CI? |
The test which received the most impact is the packet forward test:
The other chains went down a few minutes only as the other main test impacted is the The remaining very long job is for simapp v8, as it runs the highest number of tests, 86 tests but does not run forward tests. So it went down only from 1h31 to 1h25. |
Very nice, thanks for the report :) |
Closes: #4038
Description
This PR refactors the bootstrapping of n-ary chain tests in order to use the specified topology. Only linear and fully connected are currently supported, and if the topology is not specified the linear one is used.
This also facilitates the implementation of new topologies if required.
PR author checklist:
unclog
.Added tests: integration (for Hermes) or unit/mock tests (for modules).docs/
).Reviewer checklist:
Files changed
in the GitHub PR explorer.