Skip to content

Conversation

roypat
Copy link
Member

@roypat roypat commented Sep 29, 2025

Summary of the PR

Use cargo-all-features in our CI, to ensure that all feature combinations are tested. Closes #152.

The first 4 commits are fixes / cleanups, the last one actually enables cargo-all-features. Needs rust-vmm/rust-vmm-container#139 first.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

otherwise, on multi-crate workspaces, this test run does precisely
nothing.

Signed-off-by: Patrick Roy <[email protected]>
@roypat
Copy link
Member Author

roypat commented Sep 29, 2025

Note that this can deal with mutually exclusive features: https://crates.io/crates/cargo-all-features#options

ShadowCurse
ShadowCurse previously approved these changes Sep 30, 2025
@stefano-garzarella
Copy link
Member

@roypat great! thanks for doing this!

LGTM, but should we just swap the last 2 patches, I mean first update the container image, then use all-features?

roypat added 3 commits October 1, 2025 09:34
This ensures tests, binaries, benchmarks, examples and libraries are all
build-tested, instead of just the default targets.

Signed-off-by: Patrick Roy <[email protected]>
We already run `cargo build` with `RUSTFlAGS=-Dwarnings`, so also
running `cargo check` with the same arguments, on the same
architectures, and with the same rustflags gains us nothing.

Signed-off-by: Patrick Roy <[email protected]>
--libs --benches and co are already implies by --all-targets.

Signed-off-by: Patrick Roy <[email protected]>
@roypat
Copy link
Member Author

roypat commented Oct 1, 2025

@roypat great! thanks for doing this!

LGTM, but should we just swap the last 2 patches, I mean first update the container image, then use all-features?

Ah, yes, done! I also swapped the two commits about dropping the check step, and adding --all-targets to the build step, because the check step only became redundant after build also did --all-targets

ShadowCurse
ShadowCurse previously approved these changes Oct 1, 2025
roypat added 2 commits October 1, 2025 09:38
pick up container that has cargo-all-features installed.

Signed-off-by: Patrick Roy <[email protected]>
Ensure that every permutation of feature flags is both build-tested, and
has their unit-tests ran.

The unit-testing part can be reverted to `--all-features` at some point,
once we have made sure that all our cargo features are actually
additive.

Signed-off-by: Patrick Roy <[email protected]>
@roypat roypat enabled auto-merge (rebase) October 1, 2025 08:40
@roypat roypat merged commit 6f7d365 into rust-vmm:main Oct 1, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add cargo-all-features
3 participants