From 67e96ec23902b426923eb0ebe5222f0d36344e9d Mon Sep 17 00:00:00 2001 From: cargo-affected-bot <282014906+cargo-affected-bot@users.noreply.github.com> Date: Thu, 6 Aug 2026 07:21:27 +0000 Subject: [PATCH 1/2] tests: replace dead shim tripwires with a live no-coverage check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two basename-collision scenarios grep stderr for `failed to resolve binary_id` and `basename fallback ambiguous` — strings the pre-NEXTEST_BINARY_ID shim emitted and the current one does not, so five assertions were permanently true. A binary_id the shim can't resolve is a soft failure: the test lands as Skipped and collect still exits 0 on the other binary's rows, so status.success() doesn't cover it either. Assert on "produced no coverage" instead — the line collect prints for any skip. --- tests/functional/duplicate_target_names.rs | 12 ++++++++---- tests/functional/lib_bin_collision.rs | 16 +++++++++------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/tests/functional/duplicate_target_names.rs b/tests/functional/duplicate_target_names.rs index d107bc7..18d77d1 100644 --- a/tests/functional/duplicate_target_names.rs +++ b/tests/functional/duplicate_target_names.rs @@ -80,9 +80,13 @@ fn duplicate_basename_with_stripped_debuginfo_resolves_correctly() { "collect failed: stderr=\n{collect_stderr}\nstdout=\n{}", String::from_utf8_lossy(&collect.stdout) ); + // A `binary_id` the shim can't resolve is a *soft* failure: that test + // lands as `Skipped` and collect still exits 0 on whatever the other + // binary produced, so `status.success()` above doesn't cover it. + // `collect` prints this line for any skip. assert!( - !collect_stderr.contains("failed to resolve binary_id"), - "shim must not error on duplicate-basename binaries; stderr:\n{collect_stderr}" + !collect_stderr.contains("produced no coverage"), + "duplicate-basename binaries must not cost a test its coverage; stderr:\n{collect_stderr}" ); // Both crates' `builds` test must land under DISTINCT binary_ids — the @@ -161,8 +165,8 @@ fn duplicate_basename_with_stripped_debuginfo_resolves_correctly() { String::from_utf8_lossy(&recollect.stdout) ); assert!( - !recollect_stderr.contains("failed to resolve binary_id"), - "shim must not error on the post-edit collect; stderr:\n{recollect_stderr}" + !recollect_stderr.contains("produced no coverage"), + "post-edit collect must not drop a test's coverage; stderr:\n{recollect_stderr}" ); git(dir, &["checkout", "--", "mock-stub/src/lib.rs"]); diff --git a/tests/functional/lib_bin_collision.rs b/tests/functional/lib_bin_collision.rs index 17dd70e..ba92591 100644 --- a/tests/functional/lib_bin_collision.rs +++ b/tests/functional/lib_bin_collision.rs @@ -112,13 +112,15 @@ fn lib_bin_same_basename_resolves_via_nextest_binary_id() { "collect failed: stderr=\n{stderr}\nstdout=\n{}", String::from_utf8_lossy(&collect.stdout) ); + // Every test must produce coverage. A `binary_id` the shim can't resolve + // is a *soft* failure — that test lands as `Skipped` and collect still + // exits 0 on the other binary's rows — so the exit status above proves + // nothing on its own. `collect` prints this line for any skip, which is + // the only signal that distinguishes "both targets extracted" from "one + // silently dropped". assert!( - !stderr.contains("failed to resolve binary_id"), - "shim must not bail on lib+bin same-basename: stderr=\n{stderr}", - ); - assert!( - !stderr.contains("basename fallback ambiguous"), - "marker probe must disambiguate lib+bin: stderr=\n{stderr}", + !stderr.contains("produced no coverage"), + "lib+bin same-basename must not cost a target its coverage: stderr=\n{stderr}", ); // Both targets must land under their own binary_ids — nextest's @@ -151,7 +153,7 @@ fn lib_bin_same_basename_resolves_via_nextest_binary_id() { "second collect failed: {combined}" ); assert!( - !combined.contains("failed to resolve binary_id"), + !combined.contains("produced no coverage"), "second collect must not regress: {combined}", ); } From 56406d76e0840a7df765b5a3fe7fc7fd5d01d24c Mon Sep 17 00:00:00 2001 From: cargo-affected-bot <282014906+cargo-affected-bot@users.noreply.github.com> Date: Thu, 6 Aug 2026 07:31:15 +0000 Subject: [PATCH 2/2] tests: re-check the database after the second collect too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review noted the replacements are still string-greps against a collect message, so a reword would silently retire them again — the same drift this PR fixes. The first collect is cushioned by the existing SELECT DISTINCT binary_id assertions; the second collect had no DB re-check at all. Hoist those queries into per-file helpers and run them after both collects, so the property is pinned independently of the message text. --- tests/functional/duplicate_target_names.rs | 36 +++++++++++---- tests/functional/lib_bin_collision.rs | 52 +++++++++++++--------- 2 files changed, 59 insertions(+), 29 deletions(-) diff --git a/tests/functional/duplicate_target_names.rs b/tests/functional/duplicate_target_names.rs index 18d77d1..ede6954 100644 --- a/tests/functional/duplicate_target_names.rs +++ b/tests/functional/duplicate_target_names.rs @@ -66,6 +66,22 @@ fn cargo_affected_stripped(dir: &Path, args: &[&str]) -> Output { .unwrap_or_else(|e| panic!("failed to run cargo-affected: {e}")) } +/// Distinct `binary_id`s the database holds for the `builds` test. Two crates +/// each ship one, so anything other than two means the shim merged or dropped +/// a target. +fn builds_binary_ids(dir: &Path) -> Vec { + let conn = rusqlite::Connection::open(dir.join("target/affected/coverage.db")).unwrap(); + let mut stmt = conn + .prepare("SELECT DISTINCT binary_id FROM test_regions WHERE test_name = 'builds'") + .unwrap(); + let ids = stmt + .query_map([], |r| r.get(0)) + .unwrap() + .map(|r| r.unwrap()) + .collect(); + ids +} + #[test] fn duplicate_basename_with_stripped_debuginfo_resolves_correctly() { let tmp = tempfile::tempdir().unwrap(); @@ -93,15 +109,7 @@ fn duplicate_basename_with_stripped_debuginfo_resolves_correctly() { // whole point of the fix. If the shim merged them, only one binary_id // would appear (and one set of regions would silently overwrite the // other). - let db = dir.join("target/affected/coverage.db"); - let conn = rusqlite::Connection::open(&db).unwrap(); - let ids: Vec = conn - .prepare("SELECT DISTINCT binary_id FROM test_regions WHERE test_name = 'builds'") - .unwrap() - .query_map([], |r| r.get(0)) - .unwrap() - .map(|r| r.unwrap()) - .collect(); + let ids = builds_binary_ids(dir); assert_eq!( ids.len(), 2, @@ -168,6 +176,16 @@ fn duplicate_basename_with_stripped_debuginfo_resolves_correctly() { !recollect_stderr.contains("produced no coverage"), "post-edit collect must not drop a test's coverage; stderr:\n{recollect_stderr}" ); + // Re-check the database, not just stderr. The assertion above greps a + // `collect` message, so a reword of it would silently retire the guard — + // exactly the drift this scenario has already suffered once. The row + // check pins the property itself. + let ids = builds_binary_ids(dir); + assert_eq!( + ids.len(), + 2, + "post-edit collect must keep both binary_ids for `builds`, got {ids:?}" + ); git(dir, &["checkout", "--", "mock-stub/src/lib.rs"]); } diff --git a/tests/functional/lib_bin_collision.rs b/tests/functional/lib_bin_collision.rs index ba92591..ec56b26 100644 --- a/tests/functional/lib_bin_collision.rs +++ b/tests/functional/lib_bin_collision.rs @@ -98,6 +98,29 @@ fn cargo_affected_stripped(dir: &Path, args: &[&str]) -> Output { .unwrap_or_else(|e| panic!("failed to run cargo-affected: {e}")) } +/// The lib and the bin must each hold rows under their own `binary_id`. A +/// merged or dropped target loses one of the two; `when` names which collect +/// the check follows so a failure points at the right one. +fn assert_both_binary_ids(dir: &Path, when: &str) { + let conn = rusqlite::Connection::open(dir.join("target/affected/coverage.db")).unwrap(); + let mut stmt = conn + .prepare("SELECT DISTINCT binary_id FROM test_regions") + .unwrap(); + let ids: Vec = stmt + .query_map([], |r| r.get(0)) + .unwrap() + .map(|r| r.unwrap()) + .collect(); + assert!( + ids.iter().any(|id| id == "wt_perf_collide"), + "expected lib binary_id {when}, got {ids:?}", + ); + assert!( + ids.iter().any(|id| id == "wt_perf_collide::bin/wt-perf"), + "expected bin binary_id {when}, got {ids:?}", + ); +} + #[test] fn lib_bin_same_basename_resolves_via_nextest_binary_id() { let tmp = tempfile::tempdir().unwrap(); @@ -115,9 +138,9 @@ fn lib_bin_same_basename_resolves_via_nextest_binary_id() { // Every test must produce coverage. A `binary_id` the shim can't resolve // is a *soft* failure — that test lands as `Skipped` and collect still // exits 0 on the other binary's rows — so the exit status above proves - // nothing on its own. `collect` prints this line for any skip, which is - // the only signal that distinguishes "both targets extracted" from "one - // silently dropped". + // nothing on its own. `collect` prints this line for any skip. On this + // first collect the binary_id assertions below would also catch a drop; + // this line just fails earlier, with a clearer reason. assert!( !stderr.contains("produced no coverage"), "lib+bin same-basename must not cost a target its coverage: stderr=\n{stderr}", @@ -125,23 +148,7 @@ fn lib_bin_same_basename_resolves_via_nextest_binary_id() { // Both targets must land under their own binary_ids — nextest's // `` for the lib and `::bin/` for the bin. - let db = dir.join("target/affected/coverage.db"); - let conn = rusqlite::Connection::open(&db).unwrap(); - let ids: Vec = conn - .prepare("SELECT DISTINCT binary_id FROM test_regions") - .unwrap() - .query_map([], |r| r.get(0)) - .unwrap() - .map(|r| r.unwrap()) - .collect(); - assert!( - ids.iter().any(|id| id == "wt_perf_collide"), - "expected lib binary_id in {ids:?}", - ); - assert!( - ids.iter().any(|id| id == "wt_perf_collide::bin/wt-perf"), - "expected bin binary_id in {ids:?}", - ); + assert_both_binary_ids(dir, "after the first collect"); // A second collect drives the pre-run listing through the same probe // path again — confirms it stays stable run-to-run, not just on a @@ -156,4 +163,9 @@ fn lib_bin_same_basename_resolves_via_nextest_binary_id() { !combined.contains("produced no coverage"), "second collect must not regress: {combined}", ); + // Re-check the database, not just stderr. The assertion above greps a + // `collect` message, so a reword of it would silently retire the guard — + // exactly the drift this scenario has already suffered once. The row + // check pins the property itself. + assert_both_binary_ids(dir, "after the second collect"); }