Skip to content

tests: refresh duplicate-basename comments for NEXTEST_BINARY_ID shim - #31

Open
cargo-affected-bot wants to merge 3 commits into
mainfrom
docs/duplicate-target-names-comments
Open

tests: refresh duplicate-basename comments for NEXTEST_BINARY_ID shim#31
cargo-affected-bot wants to merge 3 commits into
mainfrom
docs/duplicate-target-names-comments

Conversation

@cargo-affected-bot

Copy link
Copy Markdown
Collaborator

The duplicate_target_names.rs and lib_bin_collision.rs test files document the binary-resolution mechanism in their docstrings and inline comments. Those mechanisms changed in PR #14 (shim: source binary_id from NEXTEST_BINARY_ID, drop the binary map), but the comments still describe the prior approaches:

  • The (package, target, kind) triple-based lookup that PR Disambiguate lib+bin same-basename collisions in shim marker probe #14 replaced.
  • The pre-run listing that built a binary_map — no longer exists.
  • The "byte-marker" / source-path fingerprinting fallback that depended on debug info — also no longer exists; the env variable carries the unambiguous <package>::<target> id directly.

The test scenarios themselves are still valid regression guards (duplicate basenames + stripped debuginfo + partial rebuild are real-world shapes), but the explanatory comments mislead future readers about how the current shim resolves binaries.

This PR refreshes only the comments — no code or assertion changes, no behavioural shift.

The shim now sources binary_id from NEXTEST_BINARY_ID (PR #14); these
test docstrings still describe the older triple-based lookup, the
pre-run binary_map, and the byte-marker fallback. Update comments to
match the current mechanism. No code or assertion changes.

@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.

In-diff comments are accurate against src/shim.rs. Two adjacent lines in the same files were left referring to the old shim and fit the same refresh:

  • tests/functional/duplicate_target_names.rs#L60-L61 — the cargo_affected_stripped helper docstring still attributes the RUSTFLAGS='-C debuginfo=0' to defeating "the old marker fallback". The sibling helper in lib_bin_collision.rs:88-90 already carries the up-to-date rationale (mirror the stripped-binary CI environment that originally tripped the bug).
  • tests/functional/lib_bin_collision.rs#L120-L121 — the assertion message reads "marker probe must disambiguate lib+bin", but the marker probe is gone; the !stderr.contains("basename fallback ambiguous") check itself is still a valid regression guard.

Both are out-of-diff so I haven't posted inline suggestions; happy to push a follow-up commit if you'd like them folded in.

The shim resolves binary_id from NEXTEST_BINARY_ID (shim.rs: "no marker
probe needed"); the panic message still described the defunct marker
probe. The sibling "probe path" comment was already refreshed in this PR.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@cargo-affected-bot

Copy link
Copy Markdown
Collaborator Author

Added one follow-up commit: the assert panic message at line 121 still said "marker probe must disambiguate lib+bin", referencing the same defunct mechanism this PR removes elsewhere. Updated it to "NEXTEST_BINARY_ID must disambiguate lib+bin" to match shim.rs ("no marker probe needed"). The nearby basename fallback ambiguous string check is left as-is — it's a regression guard against reintroducing the old probe path.

The cargo_affected_stripped helper docstring still attributed the
RUSTFLAGS='-C debuginfo=0' to defeating the old marker fallback. That
mechanism is gone; debuginfo=0 now just mirrors the stripped-binary CI
environment that originally tripped the duplicate-basename bug, matching
the sibling helper in lib_bin_collision.rs and this file's own header.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@cargo-affected-bot

Copy link
Copy Markdown
Collaborator Author

Pushed one more follow-up: the cargo_affected_stripped helper docstring (line 60-61) was the last spot still attributing RUSTFLAGS='-C debuginfo=0' to defeating the "old marker fallback". That mechanism is gone — refreshed it to say debuginfo=0 mirrors the stripped-binary CI environment that originally tripped the duplicate-basename bug, matching the sibling helper in lib_bin_collision.rs and this file's own header. No code or assertion changes. With this, both files are free of stale references to the pre-NEXTEST_BINARY_ID mechanisms.

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