-
Notifications
You must be signed in to change notification settings - Fork 31
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
Reorganize build/test in CI #2734
Merged
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
ss-es
changed the title
Speed up CI by splitting build and test
Reorganize build/test in CI
Mar 11, 2024
jparr721
previously approved these changes
Mar 20, 2024
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.
🥳
jparr721
approved these changes
Mar 26, 2024
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.
No linked issue.
This PR:
This is a PR to reorganize CI. Changes:
build
,build-release
andlint
jobs are removed. It is unnecessary to duplicate these on GitHub runners and on the self-hosted runners. The self-hostedtest
andbuild-arm
jobs are retained.build
job now depends ontest
passing. This means that tests run first, and that we never build if tests fail. In particular, we still do not build or push dockers if the tests fail.build-release
job runs in parallel to tests, since the artifacts from this job were not used to build dockers.test_crypto
in the regular tests, so it no longer runs as a separate job.async-std
build or test if thetokio
variant fails, and vice-versa.test
no longer runs with--no-fail-fast
. This means test failures are reported (in the form of CI failing) immediately, which should be helpful for development. This doesn't affect the regular GitHub runners, which will still continue to run all tests even after encountering a failing test, and may be helpful for debugging. (The choice of which one to run with--no-fail-fast
was completely arbitrary.)Unrelated to the primary changes, this PR also adds
DEBUG
logging forlibp2p-networking
, which is helpful when the tests inlibp2p-networking/tests/counter.rs
fail -- currently, these tests don't produce any useful logs when they fail.Notes: if we expect
build-release
artifacts to be consumed in the future, it should depend on passing tests.This PR does not:
Key places to review:
Overall test workflow, e.g. here: https://github.com/EspressoSystems/HotShot/actions/runs/8348230729?pr=2734