Skip to content

feat(cargo-coverage-gate): orchestrate coverage collection - #179

Merged
martin-kolinek merged 23 commits into
mainfrom
feat/coverage-gate-run
Sep 21, 2026
Merged

martin-kolinek merged 23 commits into
mainfrom
feat/coverage-gate-run

Conversation

@martin-kolinek

@martin-kolinek martin-kolinek commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

🤖 Adds a portable cargo coverage-gate run mode while preserving the existing evaluation CLI.

  • uses the invoking Cargo/rustc environment and validates nightly collection plus cargo-llvm-cov >= 0.9.0
  • resolves one effective target—explicit --target or rustc host—and passes it consistently through collection, reporting, and policy evaluation
  • collects locked all-features and no-default-features coverage through cargo-llvm-cov/nextest
  • accepts repeatable Cargo-style package selectors; no selectors means the workspace
  • instruments every selected package regardless of coverage threshold and evaluates valid empty LCOV normally
  • unions repeated LCOV inputs by boolean line coverage instead of adding execution counts, avoiding overflow and wraparound
  • supports explicit repeatable --no-coverage-target fallbacks that run plain nextest without claiming coverage or gating
  • delegates profile merging, object discovery (including trybuild), and filename exclusions to cargo llvm-cov report
  • writes and evaluates stable lcov-all-features.info and lcov-no-default.info artifacts directly
  • retries Windows command-line overflow with cargo-llvm-cov’s complete export arguments in an LLVM response file

The debug collector completed end-to-end on ox-tools and oxidizer with every workspace package instrumented. Upload and CI-backend behavior remain outside the tool.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9fc919b-99f7-4134-aad1-2116321b4e0c
Copilot AI lite review requested due to automatic review settings September 11, 2026 19:15
@codecov-commenter

Codecov Comments Bot (codecov-commenter) commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.6%. Comparing base (1910173) to head (895026b).
⚠️ Report is 1 commits behind head on main.

❌ Your project status has failed because the head coverage (97.6%) is below the target coverage (100.0%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@          Coverage Diff           @@
##            main    #179    +/-   ##
======================================
  Coverage   97.6%   97.6%            
======================================
  Files        304     305     +1     
  Lines      69683   70384   +701     
======================================
+ Hits       68016   68718   +702     
+ Misses      1667    1666     -1     
Flag Coverage Δ
linux 97.6% <100.0%> (+<0.1%) ⬆️
linux-arm 97.6% <100.0%> (+<0.1%) ⬆️
scheduled ?
windows 97.8% <100.0%> (+<0.1%) ⬆️

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

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Two critical and four moderate findings remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a portable cargo coverage-gate run mode for collecting, merging, publishing, and evaluating coverage while preserving the existing evaluation CLI.

Changes:

  • Adds package selection, feature configurations, and collection orchestration.
  • Integrates Cargo JSON, LLVM profile merging, LCOV publication, and response files.
  • Adds CLI integration tests, fixtures, and updated documentation.
File summaries
File Reviewed changes and final findings
crates/cargo-coverage-gate/tests/fixtures/fake-coverage-tool.rs Adds fake Cargo/LLVM tooling fixtures.
crates/cargo-coverage-gate/tests/cli.rs Adds collection and failure-path integration tests.
crates/cargo-coverage-gate/src/lib.rs Updates public documentation.
crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/run.rs Shares evaluation logic between modes.
crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/main.rs Dispatches evaluation and collection modes.
crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Implements collection. Critical (3): Windows artifact replacement uses non-replacing rename. Critical (1): nightly toolchain validation is absent. Moderate (2): newline-delimited profile paths are not validated. Moderate (1): zero-threshold uninstrumented packages are not handled. Moderate (2): aarch64-pc-windows-msvc fallback is missing. Moderate (1): --quiet does not suppress collection output.
crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/cli.rs Adds run-mode arguments and configuration.
crates/cargo-coverage-gate/README.md Updates usage documentation.
crates/cargo-coverage-gate/docs/implementation.md Documents collection implementation details.
crates/cargo-coverage-gate/docs/design/README.md Defines the collection contract.
Review details

Suppressed comments (2)

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:338

  • A selected package with min-lines-percent = 0 can be a coverage(off)/no-coverage-map crate. The existing coverage recipe explicitly removes such packages from the llvm-cov run and executes them with plain nextest (justfiles/anvil/checks/llvm-cov.just:27-36,85-94), because exporting the sole uninstrumented object fails. This path passes every explicit member to cargo llvm-cov nextest, so run --package <zero-threshold-only> fails during export instead of honoring the documented always-pass opt-out; mirror that split or otherwise handle an empty measured set.
    if selection.explicit {
        for member in &selection.members {
            command.arg("--package").arg(member.spec());
        }

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:365

  • --quiet is documented as suppressing stdout, but this branch unconditionally forwards non-JSON nextest output with println!. Therefore cargo coverage-gate run --quiet still emits nextest output (and the other collection subprocesses inherit stdout), unlike the legacy quiet mode. Thread the quiet setting through collection and suppress both forwarded and inherited child stdout when requested.
        match compiler_artifact_objects(&line) {
            Ok(artifact_objects) => objects.extend(artifact_objects),
            Err(_error) => println!("{line}"),
        }
  • Files reviewed: 10/10 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9fc919b-99f7-4134-aad1-2116321b4e0c
Copilot AI review requested due to automatic review settings September 11, 2026 22:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved moderate collection issues affect reproducibility and cross-platform correctness.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (7)

Previously missed (1) — in code that hasn't changed since the last review.

crates/cargo-coverage-gate/src/lib.rs:145

  • --coverage-dir and --jobs are single-value options in cli.rs:96-101; only --configuration and the global --package selector are repeatable. Please correct this usage text so users are not told they can repeat options that clap will not collect as lists.

crates/cargo-coverage-gate/docs/design/README.md:107

  • This contract update leaves crates/cargo-coverage-gate/docs/implementation-plans/0000.md:14-16 saying the crate has a single command, no subcommands, and is read-only, while this change adds run and writes collection artifacts. That stale implementation plan conflicts with the new CLI and its own instruction to update the document as work lands; update or supersede the plan in this change.
cargo coverage-gate [EVALUATION OPTIONS]
cargo coverage-gate run [SELECTION] [COLLECTION OPTIONS] [EVALUATION OPTIONS]

The bare command remains the backward-compatible evaluation mode. It reads one

**crates/cargo-coverage-gate/docs/implementation.md:17**
* This implementation note says no target-directory scan is needed, but the collector explicitly scans `coverage_target_dir` for `.profraw` files (`collect.rs:259,431-444`). Narrow the claim to executable-object discovery (or mention the raw-profile scan) so the implementation guide does not contradict the pipeline it describes.

cargo llvm-cov nextest --no-report run. Cargo's JSON messages provide the
executable object paths; no target-directory scan or diagnostic parsing is
needed.

**crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:334**
* `--target` is documented and used by the evaluator as the target whose package policy should be selected (`cli.rs:49-52`, `lib.rs:337-341`), but forwarding it here also changes the test build/run target. For example, `run --target x86_64-pc-windows-msvc` on a non-Windows host now requires cross-target test execution, unlike the legacy evaluation mode. Keep collection on the host or introduce/document a separate collection-target option so the existing policy-target semantics remain unambiguous.
if let Some(target) = target {
    command.arg("--target").arg(target);
**crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:524**
* This publishes with `std::fs::rename` directly to `final_lcov`, even though that destination may already exist (the new integration test creates it before a successful recollection at `tests/cli.rs:676`). Rename replacement is not portable to Windows, so an otherwise successful recollection will fail instead of atomically replacing the previous artifact on that platform. Use a platform-compatible atomic replacement strategy for an existing destination.
fs::rename(temporary_lcov.path(), final_lcov).into_app_err(format!("failed to publish LCOV file `{}`", final_lcov.display()))?;
**crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:489**
* This merge path is not portable to `aarch64-pc-windows-msvc`: the existing generated coverage recipe explicitly avoids cargo-llvm-cov there because the toolchain's `llvm-profdata` cannot merge the `.profraw` set (`crates/cargo-anvil/templates/justfiles/anvil/checks/llvm-cov.just:71-83`). The new `run` mode unconditionally performs the same merge for every target, so it fails on that platform despite documenting portable collection. Add target-specific handling or a clear unsupported-target diagnostic before this step.

fn run_profdata_merge(tools: &LlvmTools, profile_list: &Path, output: &Path) -> Result<(), AppError> {
let mut command = Command::new(&tools.profdata);
command.args(["merge", "-sparse", "-f"]).arg(profile_list).arg("-o").arg(output);
append_space_separated_env(&mut command, "LLVM_PROFDATA_FLAGS");
run_status(&mut command, "llvm-profdata merge")

**crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:305**
* This collector inherits the ambient `CARGO`/`rustc` toolchain and does not select or validate nightly Rust with `cargo-llvm-cov >= 0.7`. The crate's design explicitly requires that combination for faithful numbers because `coverage_nightly` exclusions are inactive on stable and older cargo-llvm-cov versions omit them (`crates/cargo-coverage-gate/docs/design/README.md:780-792`). As a result, the new default `run` invocation can silently gate on inflated coverage; enforce/check the requirement or make the required toolchain explicit in the `run` contract.

fn coverage_command(workspace: &WorkspaceInfo, coverage_target_dir: &Path) -> Command {
let mut command = Command::new(cargo_program());
command
.current_dir(&workspace.root)
.env("CARGO_LLVM_COV_TARGET_DIR", coverage_target_dir)
.env("CARGO_LLVM_COV_BUILD_DIR", coverage_target_dir);


- **Files reviewed:** 10/10 changed files
- **Comments generated:** 2
- **Review effort level:** Lite
</details>

Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs
Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9fc919b-99f7-4134-aad1-2116321b4e0c
Copilot AI review requested due to automatic review settings September 12, 2026 01:18
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9fc919b-99f7-4134-aad1-2116321b4e0c

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Five unresolved moderate findings must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:49

  • This ARM64 fallback returns before the policy probe below, while WorkspaceInfo::load only records package names and versions. As a result, invalid [package|workspace].metadata.coverage-gate values (for example an out-of-range threshold or malformed target policy) are never validated and the command reports success when nextest passes, despite the documented exit-2 behavior for invalid configuration (crates/cargo-coverage-gate/docs/design/README.md:111-114). Resolve the selected policies before taking this no-gate path, then skip only coverage collection/evaluation.
    if is_unsupported_arm64_windows_target(args.target.as_deref()) {
        let result =
            format!("`{ARM64_WINDOWS_TARGET}` does not support cargo-llvm-cov; tests passed without coverage collection or gating");
        run_plain_configurations(
            &workspace,

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:707

  • Valid Cargo JSON messages other than compiler-artifact are silently discarded: compiler_artifact_objects returns Ok(empty) and only JSON parse errors are printed. With --cargo-message-format=json-render-diagnostics, compiler-message records contain the compiler's rendered diagnostics; on a failed build the user is left with only the generic nonzero-status error instead of the actionable compiler message. Forward or render non-artifact diagnostic records when --quiet is not set.
        match compiler_artifact_objects(&line) {
            Ok(artifact_objects) => objects.extend(artifact_objects),
            Err(_error) if !execution.quiet => println!("{line}"),
            Err(_error) => {}

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:728

  • The new collector invokes both cargo llvm-cov nextest and plain cargo nextest through this helper, but never passes --locked. The checked-in coverage recipe uses --locked for every nextest invocation (justfiles/anvil/checks/llvm-cov.just:80, :88, and :196); without it, a stale lock can be resolved and rewritten during a coverage run, making this mode nondeterministic and unlike the existing coverage check. Add the lockfile guard here.
    command.arg(configuration.cargo_flag()).arg("--locked");

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:35

  • The documented empty --package-file no-op is checked only after resolving the toolchain and loading Cargo metadata. Consequently, an empty impact file still fails when the selected Rustup/toolchain is unusable or when the command is run outside a workspace, instead of returning success without collection; detect the empty file before toolchain/workspace initialization while still reporting file-read errors.
    let toolchain = ToolchainSelection::resolve(collection.toolchain.as_deref())?;
    let workspace = WorkspaceInfo::load(&toolchain)?;
    let selection = Selection::resolve(&workspace, &args.packages, collection.package_file.as_deref())?;
  • Files reviewed: 14/15 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Copilot AI review requested due to automatic review settings September 12, 2026 01:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved collection correctness, diagnostics, concurrency, path-handling, and documentation findings remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:708

  • Valid Cargo JSON diagnostic records are silently dropped here: compiler_artifact_objects returns Ok(Vec::new()) for every non-compiler-artifact reason, so only malformed/non-JSON lines reach println!. Because this command requests --cargo-message-format=json-render-diagnostics, a compilation failure can be reduced to the generic non-zero status while hiding rustc's rendered diagnostics. Forward compiler-message.message.rendered before continuing, as the analogous Cargo JSON consumer does in justfiles/anvil/checks/miri.just:207-209, while preserving --quiet.

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:755

  • This admits every executable field from a compiler-artifact record; it does not require the Cargo test profile. The repository's analogous Cargo JSON consumers admit only records with profile.test == true and explicitly exclude ordinary executables (justfiles/anvil/checks/miri.just:211-216, crates/cargo-anvil/docs/implementation.md:150-156). A normal binary/example artifact in this stream can therefore be passed to llvm-cov as an uninstrumented object instead of limiting export to test objects. Filter executable/filename admission by the test profile, retaining only any companion objects the export contract actually requires.
    let mut objects = BTreeSet::new();
    if let Some(executable) = message.get("executable").and_then(Value::as_str) {
        objects.insert(PathBuf::from(executable));
    }

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:455

  • The new selector matcher recursively explores both branches for every *, including repeated stars, so a user-supplied pattern such as a long run of * can cause avoidable deep/exponential work before collection starts. The existing evaluator matcher at src/verdict.rs:243-260 collapses consecutive stars; share that implementation or otherwise memoize/linearize this path.
        '*' => {
            glob_matches_from(remaining_pattern, name)
                || name
                    .split_first()
                    .is_some_and(|(_, remaining_name)| glob_matches_from(pattern, remaining_name))

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:899

  • These two arguments are constructed through Path::display(), which is lossy for non-UTF-8 paths. Because --coverage-dir is accepted as a PathBuf, a Unix path containing non-UTF-8 bytes can create the temporary files successfully but pass different paths to llvm-cov, causing export to fail; construct these arguments as OsStrings or reject such coverage directories explicitly.
        .arg(format!("-instr-profile={}", profdata.display()))
        .arg(format!("@{}", response.display()))
  • Files reviewed: 14/15 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Comment thread crates/cargo-coverage-gate/docs/design/README.md Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9fc919b-99f7-4134-aad1-2116321b4e0c
Copilot AI review requested due to automatic review settings September 12, 2026 04:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Outstanding collection, cleanup, publication, and diagnostic-handling issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:110

  • When collect_configuration fails, this ? unwinds directly and relies on TemporaryDirectory::Drop; its drop handler discards remove_dir_all errors. A failed collection can therefore leave target/coverage-gate/run-* scratch state with no warning, while cleanup errors are only combined after a completed evaluation at line 121. Route collection failures through the same cleanup/error-precedence logic (or report cleanup failure) so repeated failures do not silently accumulate.
    let mut lcov_paths = Vec::with_capacity(configurations.len());
    for configuration in configurations {
        let lcov_path = collect_configuration(&execution, configuration)?;
        lcov_paths.push(lcov_path);

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:1209

  • The ? returns before armed is cleared when temporary-file removal fails, so Drop calls remove_file a second time. This contradicts the collection contract documented in docs/implementation.md:35-36 (explicit cleanup disarms after one attempt) and differs from TemporaryDirectory::cleanup; capture the result, disarm unconditionally, then return it.
    fn cleanup(mut self) -> Result<(), AppError> {
        remove_if_present(&self.path).into_app_err(format!("failed to remove temporary file `{}`", self.path.display()))?;
        self.armed = false;

crates/cargo-coverage-gate/src/lib.rs:144

  • --coverage-dir and --jobs are scalar options (PathBuf and Option<NonZeroUsize> in cli.rs), so this says they are repeatable even though only --configuration accepts multiple values. Please document the actual option cardinalities; the README is generated from this rustdoc.
//! UTF-8 line. Use repeatable `--configuration`, `--coverage-dir`, and
  • Files reviewed: 14/15 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs
Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Comment thread crates/cargo-coverage-gate/docs/design/README.md Outdated
Comment thread crates/cargo-coverage-gate/src/lib.rs Outdated
Comment thread crates/cargo-coverage-gate/tests/cli.rs Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9fc919b-99e3-4546-847a-e30dd5cb18a4

Copilot-Session: a9fc919b-99f7-4134-aad1-2116321b4e0c

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate issues affect artifact isolation, toolchain consistency, package-file support, and zero-threshold behavior.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

crates/cargo-coverage-gate/tests/cli.rs:1017

  • The advertised zero-threshold-only plain-nextest/no-gate path is not implemented here: this test expects cargo llvm-cov nextest and report for a selection whose only package has min-lines-percent = 0. The existing coverage recipe separates such packages to plain nextest, while this PR description says run handles them that way. Either restore that routing or update the contract to explain the intentional difference.
    fake_collection_command(tmp.path(), &tools, &object)
        .env("FAKE_NO_COVERAGE_DATA", "1")
        .assert()
        .success()
        .stdout(predicate::str::contains("all packages meet their threshold"))
  • Files reviewed: 11/12 changed files
  • Comments generated: 5
  • Review effort level: Lite

Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs
Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/cli.rs
Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs
Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Comment thread crates/cargo-coverage-gate/tests/cli.rs
Declare response-file scratch state only on Windows and exclude an equivalent cleanup mutant whose replacement still executes the same Drop cleanup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9fc919b-99f7-4134-aad1-2116321b4e0c
Copilot AI review requested due to automatic review settings September 16, 2026 19:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Resolve target consistency and concurrent report publication issues, and correct the CLI documentation.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:440

  • lcov_path is the shared publication path, and run_report writes to it before this function returns it for evaluation. A concurrent run can overwrite or truncate that file between the two configurations and evaluate_paths, so this invocation can gate another selection (or read a partial report). Keep each report in invocation-private scratch for evaluation, then publish a completed copy atomically to the stable path.
    let lcov_path = execution
        .args
        .coverage_dir
        .join(format!("lcov-{}.info", configuration.artifact_name()));

crates/cargo-coverage-gate/src/lib.rs:145

  • Only --configuration is repeatable here: coverage_dir and jobs are singular PathBuf/Option<NonZeroUsize> fields in cli.rs:89-102. This rustdoc (and the generated README) currently tells callers they may repeat all three, which misstates the CLI contract; update the wording and regenerate the README.
//! workspace. Use repeatable `--configuration`, `--coverage-dir`, and `--jobs`
//! options to customize collection.
  • Files reviewed: 11/12 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Merge repeated LCOV inputs using the gate's boolean covered-line contract so anomalous execution counts cannot panic in debug builds or wrap to a false uncovered result in release builds.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9fc919b-99f7-4134-aad1-2116321b4e0c
Resolve one explicit-or-host target for collection and evaluation, and isolate concurrent collection tests with distinct consumer directories.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9fc919b-99f7-4134-aad1-2116321b4e0c
Copilot AI review requested due to automatic review settings September 17, 2026 13:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The collection path re-reads stable LCOV artifact paths for evaluation, which can yield incorrect/mixed verdicts if concurrent runs share the same --coverage-dir.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:460

  • collect_configuration returns the stable --coverage-dir/lcov-*.info path that is later re-read for evaluation. If two cargo coverage-gate run processes share the same --coverage-dir, one run can overwrite these stable artifacts between the other run’s collection and its evaluate_paths read, producing a mixed/incorrect verdict (or a transient parse error while a file is being rewritten). Consider keeping an invocation-private LCOV for evaluation (e.g., write report output to a scratch path under the isolated coverage_scratch, then copy/rename to the stable consumer path) so publication remains last-writer-wins but the in-flight verdict is stable.
fn collect_configuration(execution: &CollectionExecution<'_>, configuration: FeatureConfiguration) -> Result<PathBuf, AppError> {
    let lcov_path = execution
        .args
        .coverage_dir
        .join(format!("lcov-{}.info", configuration.artifact_name()));
  • Files reviewed: 12/13 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs
@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown

⚠️ SemVer check advisory

Inconclusive comparisons

cargo semver-checks could not complete the following comparisons. These failures are informational because an unbuildable baseline is not evidence of a breaking API change.

cargo-aprz-lib (exit 101)

     Cloning 1910173d2c5eb596341960371a2ca25957292c4d
    Building cargo-aprz-lib v1.1.2 (current)
error: running cargo-doc on crate 'cargo-aprz-lib' failed with output:
-----
   Compiling proc-macro2 v1.0.107
   Compiling unicode-ident v1.0.26
   Compiling quote v1.0.47
   Compiling libc v0.2.189
    Checking memchr v2.8.3
    Checking cfg-if v1.0.5
    Checking smallvec v1.16.1
    Checking bytes v1.12.1
    Checking regex-syntax v0.8.11
    Checking once_cell v1.21.4
   Compiling syn v3.0.6
   Compiling thiserror v2.0.20
    Checking stable_deref_trait v1.2.1
   Compiling crc32fast v1.5.2
    Checking scopeguard v1.2.0
   Compiling crossbeam-utils v0.8.23
    Checking regex-automata v0.4.18
    Checking zlib-rs v0.6.8
   Compiling parking_lot_core v0.9.12
   Compiling getrandom v0.4.3
    Checking bitflags v2.13.2
    Checking fastrand v2.5.0
    Checking gix-trace v0.1.21
    Checking lock_api v0.4.14
    Checking tinyvec v1.13.3
   Compiling serde_core v1.0.229
    Checking bstr v1.13.1
    Checking parking_lot v0.12.5
    Checking unicode-normalization v0.1.25
    Checking itoa v1.0.18
    Checking gix-validate v0.11.4
    Checking gix-utils v0.3.6
    Checking crossbeam-channel v0.5.17
    Checking same-file v1.0.6
    Checking walkdir v2.5.0
   Compiling find-msvc-tools v0.1.13
    Checking allocator-api2 v0.2.21
    Checking byteorder v1.5.0
   Compiling shlex v2.0.1
    Checking foldhash v0.2.0
    Checking equivalent v1.0.2
    Checking prodash v31.0.0
   Compiling jobserver v0.1.35
    Checking gix-error v0.2.5
   Compiling cc v1.4.7
   Compiling version_check v0.9.5
   Compiling pkg-config v0.3.34
   Compiling generic-array v0.14.7
    Checking hashbrown v0.17.1
    Checking typenum v1.20.1
   Compiling heapless v0.8.0
    Checking hash32 v0.3.1
   Compiling thiserror-impl v2.0.20
    Checking faster-hex v0.10.0
   Compiling serde v1.0.229
    Checking cpufeatures v0.2.17
   Compiling serde_derive v1.0.229
    Checking pin-project-lite v0.2.17
    Checking jiff-core v0.1.1
    Checking crypto-common v0.1.7
    Checking block-buffer v0.10.4
    Checking digest v0.10.7
    Checking sha1 v0.10.7
    Checking sha1-checked v0.10.0
    Checking gix-path v0.12.6
   Compiling synstructure v0.14.0
    Checking gix-features v0.48.1
    Checking jiff v0.2.37
   Compiling rustix v1.1.5
    Checking gix-hash v0.25.1
    Checking linux-raw-sys v0.12.1
   Compiling zerofrom-derive v0.1.8
    Checking zerofrom v0.1.8
   Compiling yoke-derive v0.8.3
    Checking gix-date v0.15.6
   Compiling autocfg v1.5.1
    Checking gix-actor v0.41.2
    Checking gix-hashtable v0.15.2
    Checking futures-core v0.3.34
    Checking gix-object v0.61.0
   Compiling zerovec-derive v0.11.6
    Checking errno v0.3.14
    Checking signal-hook-registry v1.4.8
   Compiling displaydoc v0.2.7
    Checking yoke v0.8.3
   Compiling tokio-macros v2.7.2
    Checking memmap2 v0.9.11
    Checking socket2 v0.6.5
    Checking mio v1.2.3
   Compiling cmake v0.1.58
    Checking zerovec v0.11.8
   Compiling rustversion v1.0.23
    Checking tokio v1.53.1
   Compiling fs_extra v1.3.0
   Compiling dunce v1.0.5
    Checking futures-sink v0.3.34
   Compiling aws-lc-sys v0.45.0
    Checking tinystr v0.8.4
    Checking tempfile v3.27.0
    Checking gix-fs v0.21.2
    Checking gix-chunk v0.7.3
    Checking litemap v0.8.3
   Compiling vcpkg v0.2.15
    Checking percent-encoding v2.3.2
    Checking writeable v0.6.4
    Checking icu_locale_core v2.3.0
   Compiling libz-sys v1.1.29
    Checking gix-tempfile v23.0.2
    Checking potential_utf v0.1.6
    Checking zerotrie v0.2.5
    Checking gix-quote v0.7.2
    Checking tracing-core v0.1.36
   Compiling aws-lc-rs v1.18.1
    Checking utf8_iter v1.0.4
    Checking zeroize v1.9.0
    Checking simd-adler32 v0.3.10
   Compiling icu_properties_data v2.3.0
    Checking nonempty v0.12.0
   Compiling icu_normalizer_data v2.3.0
    Checking slab v0.4.12
    Checking icu_collections v2.3.0
    Checking tracing v0.1.44
    Checking icu_provider v2.3.1
    Checking http v1.5.0
    Checking adler2 v2.0.1
    Checking fnv v1.0.7
    Checking miniz_oxide v0.9.1
    Checking gix-commitgraph v0.37.1
    Checking rustls-pki-types v1.15.1
    Checking gix-glob v0.26.1
    Checking indexmap v2.14.2
    Checking futures-io v0.3.34
    Checking log v0.4.34
    Checking futures-task v0.3.34
    Checking tokio-util v0.7.19
    Checking futures-util v0.3.34
    Checking gix-revwalk v0.32.0
    Checking http-body v1.1.0
    Checking icu_normalizer v2.3.0
    Checking icu_properties v2.3.0
    Checking flate2 v1.1.10
    Checking gix-lock v23.0.1
   Compiling num-traits v0.2.19
   Compiling syn v2.0.119
   Compiling rustls v0.23.45
   Compiling zmij v1.0.23
   Compiling httparse v1.10.1
    Checking untrusted v0.9.0
    Checking idna_adapter v1.2.2
   Compiling encoding_rs v0.8.41
    Checking futures-channel v0.3.34
    Checking atomic-waker v1.1.2
    Checking subtle v2.6.1
    Checking tower-layer v0.3.3
    Checking tower-service v0.3.3
    Checking static_assertions v1.1.0
    Checking try-lock v0.2.5
    Checking unicode-bom v2.0.3
    Checking want v0.3.1
    Checking h2 v0.4.19
    Checking idna v1.1.0
    Checking form_urlencoded v1.2.2
    Checking gix-config-value v0.18.1
    Checking core_detect v1.0.0
   Compiling multiversion_no_op v1.0.0
    Checking compression-core v0.4.33
   Compiling serde_json v1.0.151
    Checking simdutf8 v0.1.5
    Checking shell-words v1.1.1
    Checking gix-command v0.9.2
    Checking hyper v1.11.1
    Checking compression-codecs v0.4.43
    Checking url v2.5.8
    Checking kstring v2.0.4
    Checking sync_wrapper v1.0.2
    Checking gix-sec v0.14.2
    Checking ipnet v2.12.2
    Checking base64 v0.22.1
    Checking openssl-probe v0.2.1
    Checking rustls-native-certs v0.8.4
    Checking tower v0.5.3
    Checking gix-attributes v0.33.2
    Checking hyper-util v0.1.20
    Checking async-compression v0.4.48
    Checking http-body-util v0.1.5
   Compiling heck v0.5.0
   Compiling crossbeam-epoch v0.9.21
    Checking tower-http v0.6.11
    Checking gix-ref v0.64.0
    Checking gix-url v0.36.2
    Checking gix-packetline v0.21.5
    Checking filetime v0.2.29
   Compiling crossbeam-deque v0.8.8
    Checking either v1.18.0
    Checking base64 v0.23.1
    Checking mime v0.3.17
    Checking gix-prompt v0.15.1
    Checking gix-revision v0.46.0
    Checking gix-traverse v0.58.0
   Compiling zstd-sys v2.1.0+zstd.1.5.7
    Checking gix-bitmap v0.3.3
    Checking hashbrown v0.16.1
    Checking fast_time v0.1.32
   Compiling rayon-core v1.13.0
   Compiling camino v1.2.6
    Checking arrayvec v0.7.8
    Checking uluru v3.1.0
    Checking nm_impl v0.1.50
    Checking gix-index v0.52.0
    Checking gix-refspec v0.42.0
    Checking clru v0.6.3
    Checking gix-credentials v0.38.2
    Checking gix-ignore v0.21.1
   Compiling memoffset v0.9.1
    Checking semver v1.0.28
   Compiling thiserror v1.0.69
    Checking utf8parse v0.2.2
    Checking winnow v1.0.4
   Compiling cfg_aliases v0.2.2
   Compiling nix v0.30.1
    Checking toml_parser v1.1.3+spec-1.1.0
    Checking anstyle-parse v1.0.0
    Checking gix-worktree v0.53.0
    Checking gix-pack v0.71.0
    Checking nm v0.1.50
    Checking gix-config v0.57.0
    Checking gix-filter v0.31.0
    Checking gix-pathspec v0.18.1
   Compiling maybe-async v0.2.11
   Compiling thiserror-impl v1.0.69
    Checking gix-shallow v0.12.1
    Checking gix-negotiate v0.32.0
    Checking arc-swap v1.9.2
   Compiling fs-err v3.3.1
    Checking toml_datetime v1.1.1+spec-1.1.0
    Checking serde_spanned v1.1.1
    Checking smol_str v0.3.6
    Checking io-close v0.3.7
   Compiling portable-atomic v1.15.0
    Checking toml_writer v1.1.2+spec-1.1.0
    Checking anstyle v1.0.14
   Compiling zstd-safe v8.0.0
    Checking bit-vec v0.8.0
    Checking anstyle-query v1.1.5
    Checking is_terminal_polyfill v1.70.2
    Checking unicode-properties v0.1.4
    Checking bumpalo v3.20.3
    Checking colorchoice v1.0.5
   Compiling typeid v1.0.3
    Checking zopfli v0.8.3
    Checking anstream v1.0.0
    Checking ra-ap-rustc_lexer v0.160.0
error[E0080]: evaluation panicked: unicode-properties and unicode-ident must use the same Unicode version, `unicode_properties::UNICODE_VERSION` and `unicode_ident::UNICODE_VERSION` are different.
  --> /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/ra-ap-rustc_lexer-0.160.0/src/lib.rs:47:9
   |
47 | /         panic!(
48 | |             "unicode-properties and unicode-ident must use the same Unicode version, \
49 | |             `unicode_properties::UNICODE_VERSION` and `unicode_ident::UNICODE_VERSION` are \
50 | |             different."
51 | |         );
   | |_________^ evaluation of `_` failed here

For more information about this error, try `rustc --explain E0080`.
error: could not compile `ra-ap-rustc_lexer` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...

-----

error: failed to build rustdoc for crate cargo-aprz-lib v1.1.2
note: this is usually due to a compilation error in the crate,
      and is unlikely to be a bug in cargo-semver-checks
note: the following command can be used to reproduce the error:
      cargo new --lib example &&
          cd example &&
          echo '[workspace]' >> Cargo.toml &&
          cargo add --path /home/runner/work/ox-tools/ox-tools/crates/cargo-aprz-lib --features internals &&
          cargo check &&
          cargo doc

error: aborting due to failure to build rustdoc for crate cargo-aprz-lib v1.1.2

Preserve argv boundaries in rendered command diagnostics by quoting empty and whitespace-containing arguments.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9fc919b-99f7-4134-aad1-2116321b4e0c
Copilot AI review requested due to automatic review settings September 17, 2026 13:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The new collector evaluates from stable --coverage-dir LCOV paths, which can yield incorrect verdicts if concurrent runs share the same coverage directory.

Review details

Suppressed comments (1)

crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs:465

  • collect_configuration returns the published LCOV path under --coverage-dir and run later re-reads those paths for in-process evaluation. If two cargo coverage-gate run invocations share the same --coverage-dir concurrently (default target/coverage), one run can overwrite lcov-*.info between collection and evaluate_paths, leading to a verdict computed from a mixed or foreign LCOV pair.

To make the verdict deterministic under concurrency while still publishing stable artifacts, write each configuration’s LCOV into the invocation-private scratch directory (under the already-isolated CARGO_LLVM_COV_TARGET_DIR) and then copy it to the stable --coverage-dir path for consumers; evaluate using the private paths.

fn collect_configuration(execution: &CollectionExecution<'_>, configuration: FeatureConfiguration) -> Result<PathBuf, AppError> {
    let lcov_path = execution
        .args
        .coverage_dir
        .join(format!("lcov-{}.info", configuration.artifact_name()));

    run_clean(execution.workspace, execution.coverage_target_dir, execution.tools, execution.quiet)?;
    run_nextest(execution, configuration)?;
    run_report(execution, configuration, &lcov_path)?;
    Ok(lcov_path)
  • Files reviewed: 12/13 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@wukchung martinhavelka (wukchung) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Reviewed the collection pipeline against cargo-llvm-cov v0.9.0 (the enforced floor) and shell-escape. Three verified findings below, all in the new run collector. Existing open threads are not repeated.

Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Comment thread crates/cargo-coverage-gate/src/bin/cargo-coverage-gate/collect.rs Outdated
Use fresh configuration targets without workspace cleanup, normalize Windows error-206 diagnostics, and stage response-file exports before publishing completed or valid empty LCOV.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9fc919b-99f7-4134-aad1-2116321b4e0c
Copilot AI review requested due to automatic review settings September 18, 2026 14:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The PR introduces a large, cross-platform process-orchestration and Windows-fallback implementation path whose correctness and operational characteristics are difficult to fully validate from diff inspection alone.

Review details
  • Files reviewed: 12/13 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Give concurrent fake collectors independent logs and directly test Windows error-206 detection so CI and mutation results do not depend on interleaved writes or platform gating.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a9fc919b-99f7-4134-aad1-2116321b4e0c
Copilot AI review requested due to automatic review settings September 18, 2026 14:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes are internally consistent, include thorough integration/unit coverage for the new behaviors, and update the public-facing documentation/design docs to match the new contract.

Review details
  • Files reviewed: 12/13 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@martin-kolinek
martin-kolinek merged commit 3048656 into main Sep 21, 2026
29 checks passed
@martin-kolinek
martin-kolinek deleted the feat/coverage-gate-run branch September 21, 2026 10:54
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.

5 participants