test(sch): cover the sheet-pin branch of the junction reconciler - #332
Open
triglav-modular wants to merge 1 commit into
Open
test(sch): cover the sheet-pin branch of the junction reconciler#332triglav-modular wants to merge 1 commit into
triglav-modular wants to merge 1 commit into
Conversation
`attached = has_pin || has_sheet_pin` had no test for its second half — noted on mixelpixx#330 as not blocking. This covers it. The branch is only reachable one way: candidate points come from moved *symbol* pins, so the sheet-pin clause decides something only when a symbol pin vacates a point a hierarchical sheet pin also holds. The fixture puts R1's pin, the sheet's SHPIN and the dot all at (139.7, 190.5) on a wire's interior; the test removes R1 — the post-move state — and asserts the dot survives on the sheet pin alone. Verified by mutation: deleting the `|| has_sheet_pin` clause fails this test and nothing else. Its own fixture rather than an addition to junction_reconcile.kicad_sch, so the six tests already merged against that sheet keep the inputs they were written for and no existing assertion has to be adjusted. Fixture is eeschema's own output via `kicad-cli sch upgrade`. Two things KiCad decided rather than me: a sheet pin snaps to the sheet border, so the sheet is placed with its right border on the target point and sized 30.48 (24 * 1.27) to land on the grid a symbol pin snaps to; and its Sheetfile names an existing fixture, so the reference resolves rather than dangling. The helper that strips the symbol is guarded. `reconcile_junctions_at` returns its input unchanged when the sheet does not parse, which is indistinguishable from a passing sheet-pin test, so the helper asserts the symbol is gone and the parens balance. Verified that sabotaging it fails the test rather than letting it go quietly green.
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.
Summary
Follow-up to #330, closing the gap you flagged there rather than blocked on:
attached = has_pin || has_sheet_pinhad no test for its second half. Tests only — no behaviour change.Approach
The branch is reachable exactly one way. Candidate points come from moved symbol pins, so the sheet-pin clause only decides anything when a symbol pin vacates a point a hierarchical sheet pin also holds. The fixture puts R1's pin, the sheet's
SHPINand the dot all at(139.7, 190.5)on a wire's interior; the test removes R1 — the post-move state — and asserts the dot survives on the sheet pin alone.Its own fixture rather than an addition to
junction_reconcile.kicad_sch. My first attempt added a sheet to the shared one, which perturbed the inputs of the six tests already merged against it and forced edits tobulk_move_prunes_the_junction_its_pin_vacates's dot counts. Adjusting merged assertions to accommodate a new test is the wrong trade, so this leaves them untouched.Two details KiCad decided rather than me, both preserved in the committed text: a sheet pin snaps to the sheet border, so the sheet is placed with its right border on the target point and sized
30.48(24 × 1.27) to land on the grid a symbol pin snaps to — my first attempt had the pin at170.18while the symbol pin sat at139.7, silently not coincident and passing for the wrong reason. ItsSheetfilenames an existing fixture so the reference resolves rather than dangling.Compatibility and safety
None — no source behaviour changes, no public names, no schema. One new fixture file and one test plus its helper.
The helper that strips the symbol is guarded, because a silent failure here is invisible:
reconcile_junctions_atreturns its input unchanged when the sheet does not parse, which is exactly what a passing sheet-pin test looks like. It asserts the symbol is gone and the parens balance.Validation
cargo fmt --all -- --checkcargo test --workspace --locked --lib --tests— 768 pass, 1 ignoredcargo test --workspace --locked --doccargo clippy --workspace --locked --all-targets -- -D warningsTwo mutation checks, since a test that merely runs near a branch is not coverage:
|| idx.has_sheet_pin(px, py)→ this test fails, and only this testFixture is
kicad-cli sch upgradeoutput and byte-identical to a fresh rewrite, so the provenance claim is checkable.🤖 Generated with Claude Code