diff --git a/tests/functional/duplicate_target_names.rs b/tests/functional/duplicate_target_names.rs index d107bc7..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(); @@ -80,24 +96,20 @@ 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 // 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, @@ -161,8 +173,18 @@ 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}" + ); + // 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 17dd70e..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(); @@ -112,34 +135,20 @@ 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. 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("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 // `` 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 @@ -151,7 +160,12 @@ 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}", ); + // 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"); }