Skip to content

tests: make the partially-diverged scenario actually strand a test - #75

Open
cargo-affected-bot wants to merge 1 commit into
mainfrom
tests/strand-a-test-in-partial-divergence
Open

tests: make the partially-diverged scenario actually strand a test#75
cargo-affected-bot wants to merge 1 commit into
mainfrom
tests/strand-a-test-in-partial-divergence

Conversation

@cargo-affected-bot

Copy link
Copy Markdown
Collaborator

run_unions_affected_and_stranded_when_partially_diverged never stranded anything. It reset HEAD back to sha0 and asserted only combined.contains("2 tests to run") — which passed with both tests classified affected, so the affected ∪ stranded union it exists to pin had no coverage at all. Grepping tests/ for stranded turned up only comments; the stranded selection category had no functional test anywhere.

The cause is that reachability is object existence, not ancestry — relation_to_head returns Missing only when git cat-file -e fails. A git reset --hard orphans the commit but leaves it in the object database, so it stays Reachable and its tests come back as ordinary affected hits. That's the same situation run_uses_reachable_shas_when_one_sha_diverges sets up directly above, which is why the two scenarios had quietly collapsed into one.

Changes

run_unions_affected_and_stranded_when_sha_is_missing (renamed from ..._when_partially_diverged) now expires the reflog and runs git gc --prune=now after the reset, so sha1 is genuinely gone, and asserts the actual classification rather than a bare count:

  • 2 tests to run (1 affected + 0 config + 0 new + 1 stranded — one hit from each category
  • test_fa (stranded) in the per-test listing
  • the not in the repo missing-sha notice is emitted, not silently swallowed
  • a direct git cat-file -e probe confirming the prune worked, so the scenario fails loudly rather than silently reverting to the reachable case if a future git changes gc behavior

run_uses_sibling_sha_without_stranding_or_widening (renamed from run_uses_reachable_shas_when_one_sha_diverges) — nothing diverges in this scenario, and its own inline comment already said so ("So selection picks test_fa as affected, not new") while the doc comment above it claimed the opposite ("only tests stranded at the diverged sha rerun (as 'new')"). The doc comment and name now match the body, and the count assertion is tightened to 1 tests to run (1 affected + 0 config + 0 new + 0 stranded so the sibling case pins the category too — if siblings ever start stranding, this test catches it rather than passing on a matching total.

Both doc comments cross-reference each other, since the reachable/missing distinction is the only thing separating the two scenarios.

Verification

Per tests/CLAUDE.md, the new assertions were seen failing against the pre-fix behavior: removing the two gc lines and re-running gives

2 tests to run (2 affected + 0 config + 0 new + 0 stranded, 1 skipped of 3 reachable-known):
  sample_diff_partial_diverge_with_edit::a::tests::test_fa
  sample_diff_partial_diverge_with_edit::b::tests::test_fb

test result: FAILED. 0 passed; 1 failed

which is exactly the collapsed-into-affected shape described above. With the fix, the run reports 1 affected + 0 config + 0 new + 1 stranded and tags test_fa (stranded).

Full suite green locally: 112 unit + 41 functional passed, cargo clippy --all-targets and cargo fmt --check clean.

This touches tests/functional/diff_collect.rs alongside #49, but in disjoint hunks — that PR rewrites the module doc at the top of the file, this one changes two scenarios near the bottom.

@cargo-affected-bot cargo-affected-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

#49 rewrites this file's module doc, and its replacement says "a single diverged sha only strands its own tests instead of widening the run". Under the distinction this PR draws, that clause describes the missing case — a diverged-but-present sha strands nothing, which is exactly why these two scenarios had collapsed into one. The hunks are disjoint, so the two merge cleanly and leave the module doc contradicting the doc comments below it; whichever lands second is the one to reword.

@cargo-affected-bot

Copy link
Copy Markdown
Collaborator Author

Resolved on the other side, so merge order no longer matters here. #49 pushed 7a44b44 about 90 seconds after that review, replacing the diverged clause with "a sha that has dropped out of the object database strands only the tests anchored there instead of widening the run. (Divergence alone strands nothing — reachability is git cat-file -e, not ancestry.)" That's the same distinction the two scenarios here now draw, and the hunks stay disjoint, so neither PR needs a follow-up reword whichever lands second.

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.

1 participant