Skip to content

test(md_help): isolate fenced comment regression - #3923

Merged
max-sixty merged 1 commit into
mainfrom
codex/simplify-fenced-comment-test
Aug 26, 2026
Merged

max-sixty merged 1 commit into
mainfrom
codex/simplify-fenced-comment-test

Conversation

@max-sixty

Copy link
Copy Markdown
Owner

Keep the fenced HTML regression test focused on the parser behavior it owns. The previous snapshot also captured a following console block, so its expectation depended on syntax-highlighting even though highlighting was unrelated to the regression.

Snapshot a <!-- wt list --> marker directly in the feature-neutral flush renderer and remove the conditional expectations. The incident-specific guidance in tests/CLAUDE.md is no longer needed once the test contains no feature-dependent output.

Tests: cargo test --bin wt md_help; cargo test --no-default-features --features cli --bin wt md_help; cargo fmt --check; cargo insta pending-snapshots.

This was written by Codex on behalf of @max-sixty

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The test change itself holds up — I ran cargo test --bin wt md_help and cargo test --no-default-features --features cli --bin wt md_help on the merge commit and both pass, and render_markdown_flush only dims, so the new expectation is genuinely feature-neutral.

The tests/CLAUDE.md removal is the part I'd push back on. The rule that caveat was attached to — Inline snapshots over multi-assert, "when a test checks formatted output, use insta::assert_snapshot! with an inline snapshot" — is still there, unchanged, and it's what a contributor reads before writing the next styled-output snapshot. #3920's own argument was that the guidance isn't test-specific: "the information lives in nightly.yaml and Cargo.toml, neither of which a unit-test author has reason to open." That's still true after this PR — git grep -n "syntax-highlighting" -- '*.md' returns nothing once the block is gone, so the only written trace of the failure mode is a job comment in .github/workflows/nightly.yaml. The chain that produced it (#3908 merged green → nightly red on the next cron) took under 48 hours and nothing structural changed to stop a repeat; deleting the warning restores the exact state that allowed it. I've suggested a condensed three-sentence version inline if the 30 lines were the objection — the non-obvious parts are the per-configuration split (rather than gating the whole assertion, which drops the test on the one combination that catches this) and the local repro command.

Separately, and much smaller: the deleted guttered block couldn't actually detect the leak its comment claimed. format_bash_with_gutter_chopped and format_bash_with_gutter differ only when a line exceeds the available width, and format_bash_with_gutter_impl sets term_width from width_override.or_else(terminal_width).unwrap_or(usize::MAX) — under test there's no detectable width, so $ wt renders identically either way. So removing it costs nothing, but the containment it was named for (a fenced <!-- wt list --> not arming chop_next_block for the following console block) is untested now and was untested before. Worth a follow-up if you want it covered; a width-forced comparison against the unarmed rendering would assert it without baking in any colour.

Comment thread tests/CLAUDE.md
@max-sixty
max-sixty merged commit eb0432a into main Aug 26, 2026
48 checks passed
@max-sixty
max-sixty deleted the codex/simplify-fenced-comment-test branch August 26, 2026 22:06
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.

2 participants