test: suite-wide hygiene sweep — hoist imports, trim comments, merge tests#71
Merged
test: suite-wide hygiene sweep — hoist imports, trim comments, merge tests#71
Conversation
…tests
Net change: 25 files, +131 / -354. Test count drops 1812 → 1786 (-26)
because redundant one-liner tests were folded into table-style cases;
no behavioral coverage lost (per-file line-coverage unchanged).
Four categories applied uniformly per CLAUDE.md test-suite guidance:
(1) Hoist shared `use` to top of test module.
`use ratatui::Terminal; use ratatui::backend::TestBackend;` and
`use wiremock::matchers::{method, path};` style imports were
repeated in many test fns. Hoisted to the `mod tests` import
block (std → external → internal, alphabetical within each).
(2) Trim or drop rotting / verbose comments.
Deleted comments that narrate the change ("Regression: ...",
"Updated to ...", references to past behavior). CLAUDE.md global:
"That belongs in the commit message and rots in the source tree."
Multi-line rationale comments trimmed to one or two lines, leading
with the WHY. Comments explaining non-obvious invariants /
workarounds / perf-or-security considerations kept as-is.
(3) Test names. Per CLAUDE.md ("Phrase the scenario side, not the
mechanism"), shortened mechanism-leading names. Examples:
`agent_turn_text_only_response_records_assistant_message_and_returns`
→ `agent_turn_text_only_response_records_and_completes`.
(4) Merge redundant tests. CLAUDE.md: "Prefer a concise suite with
full coverage over many minimal tests. Merge tests that cover the
same path." Tiny one-liner tests calling the same fn with
different inputs collapsed into table-style tests with multiple
asserts. Two tests with near-identical setup asserting different
aspects of the same fn merged. Tests subsumed by a broader test
dropped.
Verification: cargo fmt / clippy / test green; pnpm spellcheck / lint
green. Coverage on every modified file unchanged.
Two tiny normalisations spotted in a follow-up sweep: - `bool_unset_and_empty_are_absent` → `bool_unset_or_empty_is_absent` to match the sibling `string_unset_or_empty_is_absent` (semantic OR on the input states; singular subject across both tests). - Three noop variants → the dominant `_is_a_noop`: `modal_cancel_without_snapshot_is_noop` (missing article), `swap_theme_with_unknown_name_is_silent_noop` (missing article), and `paint_*_is_a_no_op` (split spelling) in welcome.rs. The 8+ other call sites all spell it `_is_a_noop`.
Three issues surfaced by parallel review agents (code-reviewer, pr-test-analyzer, comment-analyzer): - slash/help.rs: drop the orphaned `///` on `Fake` struct (duplicated inline at the merged test) and the trait-stub asserts that crept into `display_label_combines_name_aliases_and_usage`. The stub asserts were chasing coverage rather than verifying behavior — the trait wiring is already exercised implicitly by the four display_label cases. - session/handle.rs: rephrase the antithesis tic `path SET (not just count)` → `path set so the test fails even if every snapshot points at the same file`. Global CLAUDE.md flags the X-not-Y construction unless it rules out a misconception a reader would otherwise hold; here the positive form carries the same meaning. silent-failure-hunter and pr-test-analyzer both confirmed the sweep introduces no coverage regressions; the deleted `list_finds_first_prompt_title_in_long_session` test exercised the same flat-byte-scan path as the retained `list_finds_ai_title_buried_between_head_and_tail`, with no unique branch lost.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
hakula139
added
refactor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
useimports, drop rotting / restate-the-code comments, scenario-leading test names, and merge tiny redundant tests into table-style cases.bool_unset_and_empty_are_absent→_or_empty_is_absentto match itsstring_*sibling; three noop variants (_is_noop,_is_silent_noop,_is_a_no_op) normalized to the dominant_is_a_noop.list_finds_first_prompt_title_in_long_session) exercises the same flat byte-scan path as the retainedlist_finds_ai_title_buried_between_head_and_tail, no unique branch lost.Verified by
/pr-review-toolkit:review-pr all parallel— 4 review agents (code-reviewer, pr-test-analyzer, comment-analyzer, silent-failure-hunter); follow-up commit addresses every issue raised.Changes
agent.rs_records_assistant_message_and_returns→_records_and_completes).client/anthropic/sse.rsuse wiremock::matchers::{method, path};blocks to test mod top.config.rs,config/file.rsRegression: ...narrative doc comments; replace with one-line cross-reference.session/actor.rssession/handle.rsSET (not just count)→ positive form).session/state.rsextract_user_text_*+ 6truncate_title_*+ 5parse_git_branch_*into 3 table-style tests. Trim verbose comments.session/store.rsformat_size_*and consolidate. Drop the long-session FirstPrompt test (subsumed).slash/diff.rstruncate_*+format_size_*table-style. Trim comments.slash/effort_slider.rs,picker.rs,theme.rs,init.rs,help.rs,matcher.rsinit.rsswitchesassert!(matches!(...))tolet-else + panicfor stricter mismatch reporting.slash/resume.rsuse ratatui::Terminal/TestBackend; mergeunhandled_keys_*+ctrl_other_chars_*(both verify unrecognized keys are absorbed).tui/app.rs,modal.rs,modal/list_picker.rs,theme.rs,theme/color.rs,components/{chat/blocks/assistant.rs, chat/blocks/tool/numbered_row.rs, input/popup.rs, welcome.rs}util/env.rsstring_unset_*+string_empty_*(and matchingbool_*) into 2 consolidated_unset_or_empty_is_absenttests.Test plan
cargo fmt --all --checkcargo clippy --all-targets -- -D warningscargo test --package oxide-code— 1786 passcargo llvm-covper-file — coverage unchangedpnpm spellcheckpnpm lint/pr-review-toolkit:review-pr all parallel— 4 review agents, all findings addressed