tests: add periodic sanitizer integration test pipeline#5722
tests: add periodic sanitizer integration test pipeline#5722aaron-ang wants to merge 1 commit intofirecracker-microvm:mainfrom
Conversation
aaron-ang
requested review from
Manciukic,
kalyazin,
pb8o and
xmarcalx
as code owners
aaron-ang
force-pushed
the
test/sanitizer-integration-pipeline
branch
from
cb7bf19 to
34ee078
Compare
Add a dedicated integration test that runs vmm Rust integration tests under ASAN, and wire a Buildkite pipeline generator for periodic sanitizer runs. Also document the run command and harden tools/test.sh for local runs under set -u by guarding BUILDKITE variable access. Signed-off-by: Aaron Ang <aaron.angyd@gmail.com>
aaron-ang
force-pushed
the
test/sanitizer-integration-pipeline
branch
from
34ee078 to
4de013b
Compare
ilstam
added
the
Status: Awaiting review
zulinx86
removed
the
Status: Awaiting review
| defs.LOCAL_BUILD_PATH | ||
| / "cargo_target" | ||
| / "sanitizers" | ||
| / (f"{sanitizer}-{uuid4().hex}") |
There was a problem hiding this comment.
If we don't make the directory unique, does it cause any issue?
If not, each run would place binaries into a new directory and consume its disk space, which might not be desirable on a local development environment.
There was a problem hiding this comment.
resolved. we should be able to reuse paths.
| @pytest.mark.parametrize("sanitizer", SANITIZERS) | ||
| def test_rust_integration_tests_under_sanitizer(sanitizer): | ||
| """Run vmm Rust integration tests under the specified sanitizer.""" | ||
| target_flag_var = f"CARGO_TARGET_{TARGET.upper().replace('-', '_')}_RUSTFLAGS" |
There was a problem hiding this comment.
Does this have to be specific to a target? Just RUSTFLAGS doesn't work?
There was a problem hiding this comment.
resolved. found get_rustflags helper in cargo_build.py and used RUSTFLAGS directly.
| ) | ||
| cargo( | ||
| "test", | ||
| f"--target {TARGET} -p vmm --test integration_tests", |
There was a problem hiding this comment.
Why only vmm crate? Why only integration_tests?
Can't we just all Rust tests using --workspace?
There was a problem hiding this comment.
i misread the original intent. seems like we need a bigger refactor to run all integration tests.
There was a problem hiding this comment.
If it requires much bigger refactoring, I think we can merge this change if it only covers the vmm crate. We can expand the scope later :)
|
Hi @aaron-ang , sorry for being late to review and thank you so much of your contribution! I left some comments, so could you take a look at them? |
hi, apologies for the delay. I will make the necessary edits this week. |
3 participants
Changes
tests/integration_tests/build/test_sanitizers.pyto run Rustvmmintegration tests under ASAN (address) as anoncibuild test..buildkite/pipeline_sanitizers.pyto generate Buildkite sanitizer steps per architecture (m6i.metal,m7g.metal).tests/README.mdwith sanitizer test run instructions and pipeline reference.tools/test.shto avoid unbound-variable failure outside Buildkite by changing"$BUILDKITE"to"${BUILDKITE:-false}".Reason
Closes #1662.
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
tools/devtool checkbuild --allto verify that the PR passesbuild checks on all supported architectures.
tools/devtool checkstyleto verify that the PR passes theautomated style checks.
how they are solving the problem in a clear and encompassing way.
in the PR.
CHANGELOG.md.N/A: this PR does not introduce user-facing behavior changes.
Runbook for Firecracker API changes.
N/A: this PR does not make API changes.
integration tests.
TODO.rust-vmm.