Trim

Trim

Delete by default; keep only what a reader of the site, a caller of the API, or the next maintainer would notice go missing. Losing coverage is fine when cost > value — a test that drags refactors, restates the implementation, or pins rendered output has already cost more than it protects the moment you read it. The bar is the same for code: a path, state, wrapper, or abstraction nobody's contract needs is weight the change added, not a safeguard.

Scope: user-provided; none → files touched on this branch; else the whole suite or subsystem.

The four bars

Every survivor — a function, a name, a comment, a diagnostic, a test — passes all four:

• concise — as short as the thing allows: a failure line's worth, a sentence the author cannot shorten.
• clear — one meaning, in the vocabulary the reader already holds.
• rigorous — no mechanism, helper, count, hedge, or article filling space.
• correct — states what the value is or does, and stays true after the next refactor.

Text a human reads — a diagnostic, a comment, a report — also reads friendly: skill://friendly-report.

Code

Delete on sight, in the change that exposes it:

• dead paths and redundant states the change revealed;
• obsolete indirection, and pass-through wrappers whose only remaining purpose is historical layering;
• the optional field, feature flag, fallback, retry, second entry point, or abstraction for a call site that has not arrived;
• a repeated check, defensive fallback, or error variant nothing produces — validate once, at the boundary that owns the invariant, and let every consumer downstream trust the type;
• a second representation of one concept; a derived view needs an owner and an invalidation rule, or it goes.

Prefer one authoritative path over old/new, fast/slow, cached/uncached, compatibility forks claiming the same semantics; encode the real valid states instead of a boolean combination plus optional fields. Keep performance work subordinate to semantic equivalence and measurable ownership.

Remove or consolidate

Remove on sight, without waiting for the suite to break:

• refactor drag — every design change moves it; the behaviour it pins is nobody's contract
• brittle — exact formatting, rendered wording, markdown or link syntax, hashes, timing
• redundant — same behaviour covered elsewhere → one table-driven test
• trivial assertions, no real logic
• mock-heavy, never hit code under test
• one behaviour split across siblings each re-running same setup → one test or one table
• several clauses of one behaviour welded into one test whose name needs "and" twice → clauses that matter, or nothing

An unclear owner is a delete: a test re-added after a real regression earns its place; one kept out of inertia never will.

Consolidation keeps each block's isolation: two tests each owning temp dir/config/input → merged body gives each its own. Shared temp root ⇒ second block fails "already exists" or silently asserts first block's state. Move assertions, never ownership.

Extract copy-paste setup into shared helpers where it pays off.

Deleting a test can orphan a helper, constant, or import → remove those with it. Worse reverse: NEVER delete a use because the file "looks self-contained" — traits arrive only through imports, dependent code looks fine until it runs. Before deleting an import, check every symbol it named for remaining refs.

Name the one behaviour

Test name = identifier for the one behaviour the test proves, not a sentence describing the code it runs.

Write the name before the body — it is the contract the body MUST prove; a name formed after the input describes whatever the input did. Holds for tests added mid-other-work: a feature's own new tests are not exempt.

• No scaffolding prefixes: NEVER a_, an_, the_, test_, it_, should_, when_, letters, or order-marking numbers. Test order is not a contract; a name opening like a sentence describes the case, not the behaviour.
• No filler article, leading or mid-name: a/an are prose an identifier drops — failed_command_reports_bounded_output not a_failed_command_reports_bounded_output_from_both_streams; absent_variant_misses not absent_variant_is_a_miss. the only where the verb acts on one specific thing (auto_encoding_follows_the_source).
• One clause. and/or join two values of one domain — query_and_fragment_are_refused, base_href_decides_internal_or_external — NEVER two behaviours. A second connective, a connective introducing an article (..._or_a_background), or but is refused. A name welding two clauses (segment_settles_or_retracts) splits into two tests or drops the irrelevant clause.
• State the observable result, not the mechanism: missing_package_names_each_directory not checks_package_resolution_tiers.
• Short enough to read in a failure line: aim 3–5 words. Ceiling 56 chars — the longest the suite carries today; just scripts::test-names refuses longer.
• Do not encode the body: no helper names, byte counts, file paths, or verdict endings (works, succeeds, is_ok, is_handled, passes, functions); no hedge in the outcome's place (correctly, properly, successfully, as_expected, fine, valid). handles/behaves stand in for an unstated behaviour; one word (parses) names a topic.
• Rename every test you touch or review that violates this, even if the test stays.

Naming also shrinks the suite. Open the body before renaming: what does this test fail on that no sibling fails on? Sibling already fails on it → merge (one setup, surviving assertions, one name for what the merged test proves). Nothing fails on it → delete. Then cut the name to the shortest form still stating what the body proves, dropping qualifiers it never checks; a name shortened by losing a checked behaviour was cut wrong. Search the suite for the words the name carries before writing a new one.

Shortening the suite's longest name lowers the ceiling: 56 is what the suite carries today, not a budget to spend.