Nextest: reactivate completeness #2373
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
They take a lot of memory for now, and make the CI fail from time to time even though the code works. Reviewers from now should run the completeness tests locally. It is a temporary fix to move on.
Limit the number of threads. Can be useful if they take a lot of memory
dannywillems
force-pushed
the
dw/reactivate-completeness-nextest-config
branch
from
37c0ff9
to
7eef83e
Compare
dannywillems
changed the title
dannywillems
force-pushed
the
dw/reactivate-completeness-nextest-config
branch
from
7eef83e
to
3217f1d
Compare
3 tasks
dannywillems
changed the title
dannywillems
commented
Jun 25, 2024
|
|
||
| [[profile.ci.overrides]] | ||
| filter = "test(completeness)" |
There was a problem hiding this comment.
There was a problem hiding this comment.
Nextest docs have a section on heavy tests: https://nexte.st/docs/configuration/threads-required/
This was referenced Jun 25, 2024
volhovm
approved these changes
Jun 27, 2024
|
|
||
| [[profile.ci.overrides]] | ||
| filter = "test(completeness)" |
There was a problem hiding this comment.
Nextest docs have a section on heavy tests: https://nexte.st/docs/configuration/threads-required/
| [[profile.ci.overrides]] | ||
| filter = "test(completeness)" | ||
| threads-required = 2 |
There was a problem hiding this comment.
My prediction would be that it's more optimal to give all the heavy tests all the threads (or at least half), because they're quite parallelisable anyway. But we can do it in the future when we hit the ceiling next time.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In 2372, we deactivate the tests checking the completeness. The reason was that the CI was failing from time to time with a SIGKILL (see GH actions history of 2372 and 2368 - might be others). The suspected reason was that too much memory or too much CPU was used, and the pod/container on the GH actions runner was killed. It would make sense because the completeness tests require to commit, evaluate and interpolate many polynomials.
This PR reactivate the completeness tests, by setting a particular nextest configuration only for them.
It seems that it does increase the CI runtime from 20m to 23m, unfortunately. It would make sense as we request to run on less threads. I think we should try to change some Nextest configuration parameters to see what works better.