ci: compile feature-gated code in the fast lane - #67
Merged
Conversation
The integration-tests suites were compiled nowhere but the Docker tiers, so a stale import in one of them surfaced late or not at all.
Up to standards ✅🟢 Issues
|
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
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.
Closes #59.
One fast-lane job that compiles the feature-gated suites without running them. No Docker, no test execution, no
needs:.It bit a third time, after the issue was filed
The issue records two instances on 2026-07-28. A third landed on the 0.13.0 repin branch two days later:
crates/dpp-vault/tests/audience_read_route.rs—#![cfg(feature = "integration-tests")]at line 20 — still imported the credential types fromdpp_cryptoafter 0.13.0 moved them todpp_vc.Run 30579767809 is the shape of the problem in one screenshot: eight jobs green, one red. Format check, Clippy, Unit tests, Postgres integration tests, Security audit and all three grep gates passed. The only failure was
Integration tests (testcontainers)— the Docker tier, which isneeds: [test-unit], so it starts last and would not have run at all on a machine without Docker. The fix isfix(vault): repoint credential imports in gated tests, merged in #66.Same shape as the 0.11.0 instance in the issue: a core repin moves a symbol, the default-feature workspace goes clean, and the gated suites are found one at a time afterwards.
Answering the issue's own caveat
The issue proposed
--all-featuresand flagged that it also enabless3anddesktop. The full feature set of the workspace:dpp-dal,dpp-vault,dpp-plugin-hostintegration-testsdpp-nodeintegration-tests→ pullss3dpp-plugin-hostwasm-fixture-testswasm32-wasip1target;tests/sdk_abi.rsshells out to a nestedcargo build --target wasm32-wasip1clidesktoprfd+wayland-client--all-featuresdoes not fail — I ran it, exit 0 — so the caveat's worry about a broken build is unfounded. But it compilesrfd, andcli/Cargo.tomlstates the feature is off by default precisely to keep "rfd's ashpd/Wayland stack (a large, Linux-desktop-only subtree) out of a server-side build". Switching it on in CI contradicts a decision the manifest writes down.wasm-fixture-testsis the same kind of mismatch: compiling a suite this lane can never execute.So features are named per package — the second option the issue offered.
Why one invocation rather than a loop
just check-integration, merged in #66, loopscargo test --no-runover the four crates. CI uses a singlecargo check --workspace --all-targets --features dpp-dal/…,dpp-vault/…,dpp-plugin-host/…,dpp-node/…instead.Measured locally on a warm target dir, so the two are comparable: four per-crate invocations took 24 + 35 + 17 + 45 = 121 s; the single package-qualified invocation took 27 s. Four passes re-check the shared dependency graph under four separate feature resolutions; one pass unifies it. (The
--all-featuresrun above is not in this comparison — it ran cold and its 111 s is not measuring the same thing.)The single pass is also more coverage, not less: it checks every workspace target with the gated features unified, where the loop only checks the four named crates.
Two honest differences from the local recipe:
cargo checkdoes not link, so this proves the gated tests compile, not that their binaries link; and the two commands are not identical, so they can drift. Mirroring the recipe exactly instead is a fair alternative — it just costs ~4× the wall time for the linking guarantee.Verification
Locally:
just checkgreen (586/586) and the new command exit 0 against publisheddpp-core, no[patch.crates-io]override.On CI, run 30614915331 — all ten jobs green on a clean runner, including
Feature-gated code compiles.It costs nothing in wall time. The job has no
needs:, so it runs alongside the rest of the fast lane. At 182 s on a cold cache it finishes well insideIntegration tests (testcontainers)at 291 s, which remains the critical path. The run is no longer than it was before this job existed.