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

Closed
opened 2026-09-08 00:04:47 +00:00 by david · 1 comment
Owner

Summary

Add full end-to-end main() tests in scripts/local-install.test.mjs covering the complete fj(absent/unauthenticated/ok) × rg(absent/present) install flow, including a re-run scenario proving tool-state changes between runs are picked up correctly (no stale filtering).

Background

By this point, scripts/local-install.mjs has:

  • rgAvailable() / detectFjState() detection helpers.
  • preflight() returning { rgOk, fjState } instead of hard-failing on missing rg/fj.
  • buildPackageFilters() mapping tool state to a settings.json filter shape.
  • dedupeAndRegister() applying that filter to the settings.json package entry.
  • reminder() printing a dynamic summary based on toolState.

Each of the above has unit-level test coverage from its own step. This step adds integration-level coverage: running the full main() pipeline (preflight → git pull → npm install → dedupe/register with filtering → reminder) end-to-end against the existing PATH-shim test harness, to catch any wiring mistakes between the pieces that unit tests in isolation wouldn't catch (e.g. toolState not actually threaded all the way through, or the settings file ending up with the wrong shape after a full run).

Depends on: #170 (Wire settings.json filter rewrite into dedupeAndRegister()) and #171 (Make reminder() report dynamic install/skip counts; remove dead fj/rg fail() code) — both must be complete so the full main() pipeline has all pieces wired together to test end-to-end.

Implementation Details

Extend the existing runMainAndCaptureAbort() test helper in scripts/local-install.test.mjs (or add a sibling helper) to accept withRg, withFj, fjAuthFails, fjAuthEmpty passthrough options into its underlying makeShimDir() call, alongside the existing pullFails/npmFails/etc. options.

Add the following end-to-end tests:

  1. Both fj and rg missing: main(cwd, { globalSettingsFile }) with withRg: false, withFj: false.

    • aborted === false.
    • stderr contains both skip messages (rg-skip, fj-skip).
    • stdout contains the reminder's dynamic summary with reduced counts and a skip note mentioning rg, pr-comments, and forgejo-cli.
    • globalSettings JSON has the fully-combined filtered entry (extensions: ["!extensions/pr-comments/src/index.ts", "!extensions/rg/index.ts"], skills: ["!skills/forgejo-cli"]).
  2. fj unauthenticated, rg present: withFj: true (default), fjAuthFails: true (or fjAuthEmpty: true), withRg: true.

    • aborted === false.
    • stderr contains the unauthenticated warning.
    • globalSettings entry is the plain string (no filtering).
    • stdout reminder shows full counts.
  3. Both fully available (happy path): default shim (no missing-tool flags).

    • aborted === false, no fj/rg warnings in stderr.
    • globalSettings entry is the plain string.
    • stdout reminder shows full counts.
    • (This may already be partially covered by the existing happy-path test — tighten its assertions to also check the settings entry shape and reminder counts rather than adding a fully duplicate test, if that's cleaner.)
  4. Re-run picks up fj becoming available between runs:

    • First main() call against a fresh globalSettings file with withFj: false → assert the filtered entry (excluding pr-comments/forgejo-cli).
    • Second main() call against the same globalSettings file, now with withFj: true (default, authenticated) → assert the entry is rewritten back to a plain string (assuming rg is present in both calls) — proving stale filters don't persist across runs.
    • Add the mirror case (available → then rg becomes absent) if not already covered by unit-level dedupeAndRegister tests from the earlier "Wire settings.json filter rewrite" step — check for overlap before duplicating; if the unit-level test already proves this at the dedupeAndRegister level, a lighter-weight main()-level version (asserting just the settings outcome, not every side-channel) is sufficient here.
  5. Regression — existing reminder-text assertion update: The existing test "direct invocation exits 0 and prints the Step-4 reminder on the happy path" currently asserts the old hardcoded reminder string. Update its assertion to match the new dynamic reminder format for the fully-available case (same meaning — full counts — but new exact wording per the reminder() step). Do NOT duplicate this test; update it in place.

  6. Regression — pull-failure test unaffected: Confirm "direct invocation exits non-zero when git pull fails" still passes unmodified (it fails before tool detection ever runs, since preflight happens before git pull — wait, actually preflight runs before git pull in the existing step order, so tool detection does run first; just confirm this test's assertions about the pull-failure message are unaffected by the preflight changes, since preflight succeeds in this test's shim setup by default).

Acceptance Criteria

  • runMainAndCaptureAbort() (or a new sibling helper) supports withRg, withFj, fjAuthFails, fjAuthEmpty passthrough options.
  • New end-to-end test: both fj and rg missing → full pipeline succeeds, both skip messages present, settings.json shows the combined filter, reminder shows reduced counts.
  • New end-to-end test: fj present-but-unauthenticated, rg present → full pipeline succeeds, warning present, settings.json is a plain string, reminder shows full counts.
  • Existing/tightened end-to-end test: both tools fully available → full pipeline succeeds, no warnings, settings.json is a plain string, reminder shows full counts.
  • New end-to-end test: re-running main() against the same settings file after fj becomes available removes the previously-applied filter (no stale state).
  • Updated existing test: the Step-4 reminder happy-path test's exact-string assertion matches the new dynamic reminder output.
  • Confirmed unmodified: the git-pull-failure end-to-end test still passes as-is.
  • npm test — all green, full suite (unit + end-to-end) passes with no regressions.

Test Plan

  • Run each new/updated test individually during development (node --test scripts/local-install.test.mjs -t "<test name>" or equivalent name-filtering, if supported by the Node version in use) to isolate failures.
  • Run the full suite: npm test and node --test scripts/local-install.test.mjs (direct invocation) — both green.
  • As a final sanity check for this whole multi-issue change, re-read docs/INSTALL_SCRIPT_TOOL_DETECTION_PLAN.md and docs/INSTALL_SCRIPT_TOOL_DETECTION_TEST_PLAN.md (if still present in the repo) and confirm every scenario listed there has a corresponding automated test — flag any gap found instead of silently leaving it uncovered.
## Summary Add full end-to-end `main()` tests in `scripts/local-install.test.mjs` covering the complete fj(absent/unauthenticated/ok) × rg(absent/present) install flow, including a re-run scenario proving tool-state changes between runs are picked up correctly (no stale filtering). ## Background By this point, `scripts/local-install.mjs` has: - `rgAvailable()` / `detectFjState()` detection helpers. - `preflight()` returning `{ rgOk, fjState }` instead of hard-failing on missing `rg`/`fj`. - `buildPackageFilters()` mapping tool state to a settings.json filter shape. - `dedupeAndRegister()` applying that filter to the settings.json package entry. - `reminder()` printing a dynamic summary based on `toolState`. Each of the above has unit-level test coverage from its own step. This step adds **integration-level** coverage: running the full `main()` pipeline (preflight → git pull → npm install → dedupe/register with filtering → reminder) end-to-end against the existing PATH-shim test harness, to catch any wiring mistakes between the pieces that unit tests in isolation wouldn't catch (e.g. `toolState` not actually threaded all the way through, or the settings file ending up with the wrong shape after a full run). **Depends on:** #170 (Wire settings.json filter rewrite into dedupeAndRegister()) and #171 (Make reminder() report dynamic install/skip counts; remove dead fj/rg fail() code) — both must be complete so the full `main()` pipeline has all pieces wired together to test end-to-end. ## Implementation Details Extend the existing `runMainAndCaptureAbort()` test helper in `scripts/local-install.test.mjs` (or add a sibling helper) to accept `withRg`, `withFj`, `fjAuthFails`, `fjAuthEmpty` passthrough options into its underlying `makeShimDir()` call, alongside the existing `pullFails`/`npmFails`/etc. options. Add the following end-to-end tests: 1. **Both fj and rg missing**: `main(cwd, { globalSettingsFile })` with `withRg: false, withFj: false`. - `aborted === false`. - `stderr` contains both skip messages (rg-skip, fj-skip). - stdout contains the reminder's dynamic summary with reduced counts and a skip note mentioning `rg`, `pr-comments`, and `forgejo-cli`. - `globalSettings` JSON has the fully-combined filtered entry (`extensions: ["!extensions/pr-comments/src/index.ts", "!extensions/rg/index.ts"]`, `skills: ["!skills/forgejo-cli"]`). 2. **fj unauthenticated, rg present**: `withFj: true` (default), `fjAuthFails: true` (or `fjAuthEmpty: true`), `withRg: true`. - `aborted === false`. - `stderr` contains the unauthenticated warning. - `globalSettings` entry is the plain string (no filtering). - stdout reminder shows full counts. 3. **Both fully available (happy path)**: default shim (no missing-tool flags). - `aborted === false`, no fj/rg warnings in `stderr`. - `globalSettings` entry is the plain string. - stdout reminder shows full counts. - (This may already be partially covered by the existing happy-path test — tighten its assertions to also check the settings entry shape and reminder counts rather than adding a fully duplicate test, if that's cleaner.) 4. **Re-run picks up fj becoming available between runs**: - First `main()` call against a fresh `globalSettings` file with `withFj: false` → assert the filtered entry (excluding pr-comments/forgejo-cli). - Second `main()` call against the *same* `globalSettings` file, now with `withFj: true` (default, authenticated) → assert the entry is rewritten back to a plain string (assuming `rg` is present in both calls) — proving stale filters don't persist across runs. - Add the mirror case (available → then rg becomes absent) if not already covered by unit-level `dedupeAndRegister` tests from the earlier "Wire settings.json filter rewrite" step — check for overlap before duplicating; if the unit-level test already proves this at the `dedupeAndRegister` level, a lighter-weight `main()`-level version (asserting just the settings outcome, not every side-channel) is sufficient here. 5. **Regression — existing reminder-text assertion update**: The existing test `"direct invocation exits 0 and prints the Step-4 reminder on the happy path"` currently asserts the old hardcoded reminder string. Update its assertion to match the new dynamic reminder format for the fully-available case (same meaning — full counts — but new exact wording per the `reminder()` step). Do NOT duplicate this test; update it in place. 6. **Regression — pull-failure test unaffected**: Confirm `"direct invocation exits non-zero when git pull fails"` still passes unmodified (it fails before tool detection ever runs, since preflight happens before `git pull` — wait, actually preflight runs before `git pull` in the existing step order, so tool detection *does* run first; just confirm this test's assertions about the pull-failure message are unaffected by the preflight changes, since preflight succeeds in this test's shim setup by default). ## Acceptance Criteria - [ ] `runMainAndCaptureAbort()` (or a new sibling helper) supports `withRg`, `withFj`, `fjAuthFails`, `fjAuthEmpty` passthrough options. - [ ] New end-to-end test: both fj and rg missing → full pipeline succeeds, both skip messages present, settings.json shows the combined filter, reminder shows reduced counts. - [ ] New end-to-end test: fj present-but-unauthenticated, rg present → full pipeline succeeds, warning present, settings.json is a plain string, reminder shows full counts. - [ ] Existing/tightened end-to-end test: both tools fully available → full pipeline succeeds, no warnings, settings.json is a plain string, reminder shows full counts. - [ ] New end-to-end test: re-running `main()` against the same settings file after fj becomes available removes the previously-applied filter (no stale state). - [ ] Updated existing test: the Step-4 reminder happy-path test's exact-string assertion matches the new dynamic reminder output. - [ ] Confirmed unmodified: the git-pull-failure end-to-end test still passes as-is. - [ ] `npm test` — all green, full suite (unit + end-to-end) passes with no regressions. ## Test Plan - Run each new/updated test individually during development (`node --test scripts/local-install.test.mjs -t "<test name>"` or equivalent name-filtering, if supported by the Node version in use) to isolate failures. - Run the full suite: `npm test` and `node --test scripts/local-install.test.mjs` (direct invocation) — both green. - As a final sanity check for this whole multi-issue change, re-read `docs/INSTALL_SCRIPT_TOOL_DETECTION_PLAN.md` and `docs/INSTALL_SCRIPT_TOOL_DETECTION_TEST_PLAN.md` (if still present in the repo) and confirm every scenario listed there has a corresponding automated test — flag any gap found instead of silently leaving it uncovered.
david closed this issue 2026-09-08 03:30:53 +00:00
Author
Owner

pi-loop opened and merged a pull request for this issue: #181

pi-loop opened and merged a pull request for this issue: https://git.excelera.net/david/pi-extensions-and-skills/pulls/181
Sign in to join this conversation.
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
david/pi-extensions-and-skills#172
No description provided.