172: Add end-to-end main() tests for the fj/rg tool-state matrix #181

Merged

david merged 2 commits from feature/issue-172/add-end-to-end-main-tests-for-the-fj-rg-tool-state into main

2026-09-08 03:30:53 +00:00

david commented

2026-09-08 03:30:52 +00:00

Owner

172: Add end-to-end main() tests for the fj/rg tool-state matrix

Implements 172.

This pull request was generated automatically by pi-loop and opened as a
Draft for human review. Do not merge without reviewing the changes.

Generated in 25m 13s.

Review findings

Now let's view the section around line 1583 (the matrix tests) to add the missing combos and fix the comments.Now let's add the two missing tool-state combinations and update comments. First, let's craft the new tests to insert after the "no warnings when both fully available" test, before the re-run tests (grouping all steady-state combos together).Now I'll insert the two missing combos right after the "both fully available" test.Now update the JSDoc comment mismatch and the Low finding note about relative-path matching:Now run the tests to verify everything passes.All new tests pass, and the two pre-existing failures remain as documented (unrelated to this change). Let's confirm the diff looks correct.Both findings are addressed: the Medium finding (missing rg-absent combos) now has two new tests covering rg-absent + fj-ok and rg-absent + fj-unauthenticated, plus an updated comment clarifying full 6-combo coverage across the block. The Low finding (missing cross-reference) now has a note pointing to the real-pi-style relative-path test.Confirmed: no lint/build scripts exist; npm test is the only gate, and it's green except for the two pre-existing failures already confirmed unrelated to this change.

Review Findings — 172

Summary

Both findings resolved. Added two new main()-level end-to-end tests covering the previously-missing tool-state combinations (rg-absent + fj-ok, rg-absent + fj-unauthenticated), giving genuine 6/6 coverage of the { rgOk, fjState } matrix across the block (the remaining 2 combos — both-ok, and rg-ok+fj-absent — are covered by existing tests). Updated the section comment to describe the actual coverage accurately instead of overclaiming. Added a cross-reference comment pointing to the real-pi-style relative-path test so reviewers don't need to hunt for where relative-path matching is verified. Re-ran npm test: all 127 tests pass except the same 2 pre-existing failures (macOS tmp-path regex mismatch and mongodb manifest assertion) that were already confirmed unrelated to this change and reproduce identically on the parent commit. No lint or build scripts exist in this repo — npm test is the only gate.

Critical

(none)

High

(none)

Medium

scripts/local-install.test.mjs:1585 — The "full fj/rg tool-state matrix" comment (line 1583) implies exhaustive coverage of the 3×2 fj/rg state combinations through main(), but only 3 combinations are exercised directly by the new block (both-absent, fj-unauthenticated+rg-ok, both-ok) plus 2 single-transition re-run tests (fj-absent→ok, rg-ok→absent). The "rg-absent + fj-ok" and "rg-absent + fj-unauthenticated" combinations are never driven through main() (only through the lower-level dedupeAndRegister/buildPackageFilters unit tests). Suggested fix: either add the two missing combinations as main()-level tests for true end-to-end parity, or soften the section comment/rename it to avoid overclaiming exhaustiveness (e.g. "main: representative fj/rg tool-state scenarios (end-to-end)").

Low

scripts/local-install.test.mjs:351-358 — The comment explains why the settings file is pre-seeded with a plain-string entry pointing at cwd, but doesn't explain why resolve(cwd) (an absolute path) is used rather than the real pi install behavior of writing a path relative to the settings directory (which rewritePackageEntry's matching logic specifically accounts for, per its own doc comment at scripts/local-install.mjs:~440). This mismatch is intentional and mirrored from existing dedupeAndRegister tests, but a one-line cross-reference to the "main end-to-end applies the filter rewrite against a real-pi-style settings write" test (scripts/local-install.test.mjs:1164) would make clear that relative-path matching is covered elsewhere, so a reviewer doesn't need to go hunting to confirm the abs-path shortcut isn't hiding a real gap. Suggested fix: add a short "(relative-path matching is covered separately by the real-pi-style test above)" note to the comment.

## 172: Add end-to-end main() tests for the fj/rg tool-state matrix Implements [172](https://git.excelera.net/david/pi-extensions-and-skills/issues/172). > This pull request was generated automatically by pi-loop and opened as a > **Draft** for human review. Do not merge without reviewing the changes. Generated in 25m 13s. ## Review findings Now let's view the section around line 1583 (the matrix tests) to add the missing combos and fix the comments.Now let's add the two missing tool-state combinations and update comments. First, let's craft the new tests to insert after the "no warnings when both fully available" test, before the re-run tests (grouping all steady-state combos together).Now I'll insert the two missing combos right after the "both fully available" test.Now update the JSDoc comment mismatch and the Low finding note about relative-path matching:Now run the tests to verify everything passes.All new tests pass, and the two pre-existing failures remain as documented (unrelated to this change). Let's confirm the diff looks correct.Both findings are addressed: the Medium finding (missing rg-absent combos) now has two new tests covering `rg-absent + fj-ok` and `rg-absent + fj-unauthenticated`, plus an updated comment clarifying full 6-combo coverage across the block. The Low finding (missing cross-reference) now has a note pointing to the real-pi-style relative-path test.Confirmed: no lint/build scripts exist; `npm test` is the only gate, and it's green except for the two pre-existing failures already confirmed unrelated to this change. # Review Findings — 172 ## Summary Both findings resolved. Added two new `main()`-level end-to-end tests covering the previously-missing tool-state combinations (`rg-absent + fj-ok`, `rg-absent + fj-unauthenticated`), giving genuine 6/6 coverage of the `{ rgOk, fjState }` matrix across the block (the remaining 2 combos — both-ok, and rg-ok+fj-absent — are covered by existing tests). Updated the section comment to describe the actual coverage accurately instead of overclaiming. Added a cross-reference comment pointing to the real-pi-style relative-path test so reviewers don't need to hunt for where relative-path matching is verified. Re-ran `npm test`: all 127 tests pass except the same 2 pre-existing failures (macOS tmp-path regex mismatch and mongodb manifest assertion) that were already confirmed unrelated to this change and reproduce identically on the parent commit. No lint or build scripts exist in this repo — `npm test` is the only gate. ## Critical (none) ## High (none) ## Medium - [x] scripts/local-install.test.mjs:1585 — The "full fj/rg tool-state matrix" comment (line 1583) implies exhaustive coverage of the 3×2 fj/rg state combinations through `main()`, but only 3 combinations are exercised directly by the new block (both-absent, fj-unauthenticated+rg-ok, both-ok) plus 2 single-transition re-run tests (fj-absent→ok, rg-ok→absent). The "rg-absent + fj-ok" and "rg-absent + fj-unauthenticated" combinations are never driven through `main()` (only through the lower-level `dedupeAndRegister`/`buildPackageFilters` unit tests). Suggested fix: either add the two missing combinations as `main()`-level tests for true end-to-end parity, or soften the section comment/rename it to avoid overclaiming exhaustiveness (e.g. "main: representative fj/rg tool-state scenarios (end-to-end)"). ## Low - [x] scripts/local-install.test.mjs:351-358 — The comment explains why the settings file is pre-seeded with a plain-string entry pointing at `cwd`, but doesn't explain why `resolve(cwd)` (an absolute path) is used rather than the real `pi install` behavior of writing a path relative to the settings directory (which `rewritePackageEntry`'s matching logic specifically accounts for, per its own doc comment at scripts/local-install.mjs:~440). This mismatch is intentional and mirrored from existing `dedupeAndRegister` tests, but a one-line cross-reference to the `"main end-to-end applies the filter rewrite against a real-pi-style settings write"` test (scripts/local-install.test.mjs:1164) would make clear that relative-path matching is covered elsewhere, so a reviewer doesn't need to go hunting to confirm the abs-path shortcut isn't hiding a real gap. Suggested fix: add a short "(relative-path matching is covered separately by the real-pi-style test above)" note to the comment.

david added 2 commits

2026-09-08 03:30:52 +00:00