Repository navigation
F5: review and trim the card's tests, fix what the review found, add test-review skills - #675
Merged
Merged
Conversation
passcod
changed the title
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Delete tests the type system or a parser dependency already covers, fold single-assertion tests into the neighbour that shares their setup, and move a jobs test onto the database query it exercises. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
The removal history test aged audit entries with an UPDATE the audit guard refuses and into a week with no partition. Create that week's partition and lift the guard for the update. The working doc records what applying lens A and B found and the Vitest and Playwright findings. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
A lease the sweeper released before any run read it has no releasing operator, so a late read was told it had expired. The refusal now names the reason. The working doc also records lens C's design decisions and what checking its suspected defects found. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
A variant-list macro with round-trip tests, a shared PyYAML scalar fixture, shared variable rules between set and import, one table for the maintenance window refusals, and helper extraction in the Ansible Playwright spec. Co-Authored-By: Claude <noreply@anthropic.com>
cargo-llvm-cov over the RAM-backed Postgres, so a merged or deleted test can be checked against what the rest of the suite still covers. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…se a regex in a limit exclusion Rows for branches of the Ansible code nothing reached, the stateful endpoint families folded into scenario tests, and the pure tables that add rows. A limit such as `!~name-.*` is refused like `~name-.*` instead of silently excluding nothing. Co-Authored-By: Claude <noreply@anthropic.com>
…cord unread give-ups as unstarted An unreachable repository no longer replaces a verdict that the published copy does not read, so the alert stands until a good read. An operator giving up a lease no run has read records the same reason as the sweep. A limit's name is trimmed after an exclusion or intersection mark, and a top-level non-scalar key names the top level. Co-Authored-By: Claude <noreply@anthropic.com>
…rs (lens D round 2) Round 2 of the test consolidation review: merges and deletions across the database, jobs, endpoint and e2e tests, each checked for the assertions and branches it keeps. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…lenses Lens D's third round found nothing worth acting on in the card's own tests. It noted that no test swept an expired lease with a bound window; that step is added, and fails if the sweep stops ending the window. The working doc scores each lens. Co-Authored-By: Claude <noreply@anthropic.com>
…doc and a coverage-diff recipe The process from the test review of this card, as two skills over one reference doc, with a committed script that lists production functions a coverage run no longer reaches. The working doc records the scoring of each lens and the shape decisions. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…from main, drop the version from the new skills After rebasing onto main, restore the history tests the other cards added and port their run-state assertions into the folded tests. The history now lists a removal where it happened, so the folded test expects that order. The test-quality doc and the two skills carry nothing specific to a card or its numbers, and no workhorse-version.
passcod
force-pushed
the
workhorse/f5-5
branch
from
October 23, 2026 11:11
f8fc5cf to
7d7699c
Compare
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
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.
Follow-up to #672
What this PR does
This follow-up does not add new product behaviour on top of the inventory import button; it is a quality pass over the large test suite that #672 (and the other PRs for card F5) introduced:
test-quality.mddoc and two new agent skills (review-tests,write-good-tests) that codify a repeatable process for cutting trivial, dependency-testing, or sprawling tests down to ones that actually protect against regressions.database,private-server, andprivate-web: consolidating narrow per-case tests into table-driven or exhaustive-match tests, removing assertions that couldn't fail, and folding duplicated setups together, without reducing coverage.ansible/catalogue.rs,ansible/held.rs,inventory_file.rs,limit.rs,ansible_variables.rs) where the test consolidation uncovered genuine duplication.commons-typeshelpers (variants.rs,rank.rsadditions) used to make enum/struct coverage drift-proof.scripts/coverage-fn-diff.shand matchingjustfiletargets so a coverage baseline can be diffed against a later run to prove no production function lost coverage during the test cleanup.Test plan
database,private-server,commons-types,jobs) underjust coverageand confirmed viajust coverage-diffthat no production function dropped out of coverage (only folded test functions and one generic instance appear in the diff).just test-webandjust typecheck/just checkfor the private-web changes, plus the touched Playwright specs (ansible.spec.ts,groups.spec.ts,maintenance.spec.ts) and Vitest files for the ansible components.🦸 Review Hero