Improve dependency management between buildkite steps #4803
Conversation
This file contains hidden or 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
`test_dependencies.py` only contained a single test, which tested the licences of the dependencies. It only ran on Intel. Thus, it was a style check in all but name. Move it to `test_licences.py`, which is where it belongs. Signed-off-by: Patrick Roy <[email protected]>
The `return True` was flagged by IntelliJ as unreachable (and I concur), the main function was probably to allow running the test without pytest, but we have separate mechanism for this these days (`./tools/devtool checkstyle`). Signed-off-by: Patrick Roy <[email protected]>
The idea here is that these tests depend on the compilation step, and thus test a production binary. This smells "integration" to me. The actual reason for moving this however is so that we can have the `build` group no longer depend on the shared compilation step. Signed-off-by: Patrick Roy <[email protected]>
The point of these is to run in parallel with the compilation steps, and not block the wait step if they fail. However, since we use `depends_on: null` for these, it doesn't matter whether they are initial or not, so just add them via `add_step` with `decorate=False`, as we do for normal steps. Signed-off-by: Patrick Roy <[email protected]>
No tests in the build group use the pre-compiled binaries, as these tests only run clippy, compute code coverage, and run unittests. So just start running these immediately. Signed-off-by: Patrick Roy <[email protected]>
Store whether a given instances is x86 or arm Signed-off-by: Patrick Roy <[email protected]>
roypat
force-pushed
the
buildkite-dependencies
branch
from
291ac04 to
47dfba2
Compare
Reduces code duplication slightly Signed-off-by: Patrick Roy <[email protected]>
roypat
force-pushed
the
buildkite-dependencies
branch
from
47dfba2 to
f0fa22e
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4803 +/- ##
=======================================
Coverage 84.32% 84.32%
=======================================
Files 249 249
Lines 27521 27521
=======================================
Hits 23206 23206
Misses 4315 4315
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
roypat
force-pushed
the
buildkite-dependencies
branch
from
f0fa22e to
2c05520
Compare
pb8o
reviewed
Sep 16, 2024
9 tasks
These will allow us to have x86 tests only block on x86 compilation, and arm tests only on arm compilation. Signed-off-by: Patrick Roy <[email protected]>
This way, if the x86 build is faster than the ARM build, the x86 tests can start running while the ARM ones are still blocked. Signed-off-by: Patrick Roy <[email protected]>
Name more clearly indicate what the parameter does (adding `depends_on` steps for the shared compilation, and adapting the step commands with an artifact download). Signed-off-by: Patrick Roy <[email protected]>
All other args are passed down as `**kwargs` to `group`, so call out which ones aren't. Signed-off-by: Patrick Roy <[email protected]>
roypat
force-pushed
the
buildkite-dependencies
branch
from
2c05520 to
353e8af
Compare
roypat
added
the
Status: Awaiting review
pb8o
approved these changes
Sep 16, 2024
zulinx86
approved these changes
Sep 16, 2024
roypat
added a commit
to roypat/firecracker
that referenced
this pull request
Sep 16, 2024
These were needed when the we still had a `wait` step (to overwrite the `wait` step and instead start immediately), however since firecracker-microvm#4803 we do not have a `wait` step anymore (instead everything that used to be after the wait step now explicitly depends on the compilation steps). Thus, having the `depends_on: null` entries for kani and style steps is no longer needed (it also does not harm, but it looks ugly in buildkite's canvas view). Signed-off-by: Patrick Roy <[email protected]>
roypat
added a commit
that referenced
this pull request
Sep 16, 2024
These were needed when the we still had a `wait` step (to overwrite the `wait` step and instead start immediately), however since #4803 we do not have a `wait` step anymore (instead everything that used to be after the wait step now explicitly depends on the compilation steps). Thus, having the `depends_on: null` entries for kani and style steps is no longer needed (it also does not harm, but it looks ugly in buildkite's canvas view). Signed-off-by: Patrick Roy <[email protected]>
RiverPhillips
pushed a commit
to RiverPhillips/firecracker
that referenced
this pull request
Sep 20, 2024
These were needed when the we still had a `wait` step (to overwrite the `wait` step and instead start immediately), however since firecracker-microvm#4803 we do not have a `wait` step anymore (instead everything that used to be after the wait step now explicitly depends on the compilation steps). Thus, having the `depends_on: null` entries for kani and style steps is no longer needed (it also does not harm, but it looks ugly in buildkite's canvas view). Signed-off-by: Patrick Roy <[email protected]> Signed-off-by: River Phillips <[email protected]>
ShadowCurse
pushed a commit
to ShadowCurse/firecracker
that referenced
this pull request
Nov 5, 2024
These were needed when the we still had a `wait` step (to overwrite the `wait` step and instead start immediately), however since firecracker-microvm#4803 we do not have a `wait` step anymore (instead everything that used to be after the wait step now explicitly depends on the compilation steps). Thus, having the `depends_on: null` entries for kani and style steps is no longer needed (it also does not harm, but it looks ugly in buildkite's canvas view). Signed-off-by: Patrick Roy <[email protected]>
ShadowCurse
pushed a commit
to ShadowCurse/firecracker
that referenced
this pull request
Nov 5, 2024
These were needed when the we still had a `wait` step (to overwrite the `wait` step and instead start immediately), however since firecracker-microvm#4803 we do not have a `wait` step anymore (instead everything that used to be after the wait step now explicitly depends on the compilation steps). Thus, having the `depends_on: null` entries for kani and style steps is no longer needed (it also does not harm, but it looks ugly in buildkite's canvas view). Signed-off-by: Patrick Roy <[email protected]>
ShadowCurse
pushed a commit
to ShadowCurse/firecracker
that referenced
this pull request
Nov 5, 2024
These were needed when the we still had a `wait` step (to overwrite the `wait` step and instead start immediately), however since firecracker-microvm#4803 we do not have a `wait` step anymore (instead everything that used to be after the wait step now explicitly depends on the compilation steps). Thus, having the `depends_on: null` entries for kani and style steps is no longer needed (it also does not harm, but it looks ugly in buildkite's canvas view). Signed-off-by: Patrick Roy <[email protected]>
ShadowCurse
pushed a commit
to ShadowCurse/firecracker
that referenced
this pull request
Nov 5, 2024
These were needed when the we still had a `wait` step (to overwrite the `wait` step and instead start immediately), however since firecracker-microvm#4803 we do not have a `wait` step anymore (instead everything that used to be after the wait step now explicitly depends on the compilation steps). Thus, having the `depends_on: null` entries for kani and style steps is no longer needed (it also does not harm, but it looks ugly in buildkite's canvas view). Signed-off-by: Patrick Roy <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Changes
buildgroup not block on the shared compilation step (by moving the 2 tests that require the shared artifacts to the functional group)Reason
my ocd, mostly
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.PR Checklist
PR.
CHANGELOG.md.TODOs link to an issue.contribution quality standards.
rust-vmm.