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

Fix optional PR tests #4705

Merged
merged 1 commit into from
Jul 26, 2024
Merged

Fix optional PR tests #4705

merged 1 commit into from
Jul 26, 2024

Conversation

bchalios
Copy link
Contributor

@bchalios bchalios commented Jul 25, 2024

Changes

Change no block pipelines to not use the build step

Reason

No block pipeline runs rust benchmarks (among other things) which need to run cargo bench to build those benchmarks. Problem is that when we use pre-build artifacts some build paths don't match leading to errors like this: https://buildkite.com/firecracker/firecracker-pr-optional/builds/8136#0190dfc2-bdd9-42fc-9471-91b816f43d3f

Since we anyway rebuild these artifacts just do it in the test step and drop the build step.

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

  • If a specific issue led to this PR, this PR closes the issue.
  • The description of changes is clear and encompassing.
  • Any required documentation changes (code and docs) are included in this
    PR.
  • API changes follow the Runbook for Firecracker API changes.
  • User-facing changes are mentioned in CHANGELOG.md.
  • All added/changed functionality is tested.
  • New TODOs link to an issue.
  • Commits meet
    contribution quality standards.

  • This functionality cannot be added in rust-vmm.

Copy link

codecov bot commented Jul 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.83%. Comparing base (e03f723) to head (b294af9).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4705   +/-   ##
=======================================
  Coverage   82.83%   82.83%           
=======================================
  Files         254      254           
  Lines       30792    30792           
=======================================
  Hits        25508    25508           
  Misses       5284     5284           
Flag Coverage Δ
5.10-c5n.metal 82.93% <ø> (ø)
5.10-m5n.metal 82.91% <ø> (-0.01%) ⬇️
5.10-m6a.metal 82.24% <ø> (+<0.01%) ⬆️
5.10-m6g.metal 80.04% <ø> (ø)
5.10-m6i.metal 82.92% <ø> (ø)
5.10-m7g.metal 80.04% <ø> (ø)
6.1-c5n.metal 82.93% <ø> (ø)
6.1-m5n.metal 82.91% <ø> (-0.01%) ⬇️
6.1-m6a.metal 82.23% <ø> (ø)
6.1-m6g.metal 80.04% <ø> (+<0.01%) ⬆️
6.1-m6i.metal 82.91% <ø> (-0.01%) ⬇️
6.1-m7g.metal 80.03% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bchalios bchalios force-pushed the fix_ab branch 4 times, most recently from 0159b8f to 7ab1ae3 Compare July 25, 2024 14:41
@bchalios bchalios marked this pull request as ready for review July 25, 2024 16:15
@bchalios bchalios added the Status: Awaiting review Indicates that a pull request is ready to be reviewed label Jul 25, 2024
.buildkite/common.py Outdated Show resolved Hide resolved
Optional pipeline started failing during a build failure in the rust
benchmarks tests.

During the build step, we build REVISION_A (main) and REVISION_B (tip of
the PR commit) and pass it over to the subsequent pipeline steps that
run the test.

However, 'test_benchmarks.py' needs to build the benchmarks, running
`cargo bench`. That step started failing when building 'aws-lc-rs-sys'
because CMake is complaining that its CMakeCache.txt file is not in the
same path as the one initially created.

Since we anyway run `cargo bench` in the test there's not really need to
pre-build firecracker. So, modify the buildkite pipeline logic to allow
defining pipelines without the build step, and change the
pipeline_pr_no_block.py to not use it.

Signed-off-by: Babis Chalios <bchalios@amazon.es>
@zulinx86 zulinx86 merged commit 2476009 into firecracker-microvm:main Jul 26, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Awaiting review Indicates that a pull request is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants