[PR #849] [MERGED] test: eliminate fixture races and prune redundant scaffolding #2287

New issue

Closed

opened 2026-10-16 21:16:46 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:16:46 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/849
Author: @morluto
Created: 10/7/2026
Status: ✅ Merged
Merged: 10/7/2026
Merged by: @morluto

Base: main ← Head: cleanup/test-suite-races

📝 Commits (2)

116e510 test: eliminate fixture races and prune redundant scaffolding
57e4a31 fix(test): publish launcher capture readiness without renaming

📊 Changes

22 files changed (+421 additions, -719 deletions)

View changed files

📝 .github/workflows/ci.yml (+1 -1)
➖ docs/test-suite-audit.md (+0 -169)
📝 docs/testing.md (+0 -3)
📝 src/domain/validationTaxonomy.test.ts (+1 -36)
📝 tests/boundary/browser/browserProcessStartup.test.ts (+19 -20)
📝 tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts (+6 -5)
📝 tests/boundary/browser/webSourceMapResponseCleanup.test.ts (+10 -16)
📝 tests/boundary/filesystem/interfaceBuilderBundle.test.ts (+12 -21)
📝 tests/boundary/filesystem/nibNativeHierarchy.test.ts (+8 -5)
📝 tests/boundary/filesystem/setupConfig.test.ts (+22 -18)
📝 tests/boundary/mcp/mcpDoctor.test.ts (+21 -20)
📝 tests/boundary/providers/ghidra/ghidraClient.test.ts (+34 -17)
📝 tests/boundary/providers/ghidra/ghidraLauncher.test.ts (+54 -62)
📝 tests/boundary/providers/hopper/hopperClient.cleanup.test.ts (+13 -56)
➕ tests/boundary/providers/hopper/hopperClient.fixture.ts (+112 -0)
📝 tests/boundary/providers/hopper/hopperClient.progress.test.ts (+35 -79)
📝 tests/boundary/providers/hopper/hopperClient.protocol.test.ts (+35 -79)
📝 tests/boundary/providers/hopper/hopperClient.restart.test.ts (+6 -67)
📝 tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts (+1 -41)
📝 tests/fixtures/captureGhidraLaunch.mjs (+5 -1)

...and 2 more files

📄 Description

Summary

Address the test-suite audit findings with deterministic synchronization, shared fixture ownership, targeted pruning, and correct project placement.

Fix the CI-observed Ghidra capture race: publish a readiness marker only after closing the complete JSON capture, then validate it before asserting its fields. This also respects Windows private-runtime handles that restrict renames.
Replace Hopper/Ghidra sleep-based coordination with actual process/request acknowledgements and explicit response gates over test-only IPC. The late-reply case now delivers a real late reply; progress is asserted before releasing the terminal response.
Advance deadline timers only after real fixture startup, retain process/filesystem I/O, and remove the cancellation test's 500 ms host-speed assertion.
Await browser fixture termination and socket closure; consolidate five copied Hopper launchers and redundant temporary-root cleanup scaffolding.
Replace incidental frame-tree counts, option positions, and fixed request IDs with behavioral and correlation assertions. Report unavailable UID support as an explicit skip.
Remove the handwritten taxonomy-disjointness test; retain both empty-inventory failure-channel regressions.
Move Interface Builder filesystem/compiler tests from the service project to tests/boundary/filesystem, invoke ibtool asynchronously, and update the macOS CI command. All native hierarchy/storyboard assertions remain.
Delete the one-off docs/test-suite-audit.md artifact and its link from docs/testing.md; record decisions and verification here instead.

Contract and boundary impact

Test fixtures, test placement, CI routing, and documentation only. Production behavior, CLI/MCP schemas, catalog metadata, test retries, and coverage thresholds are unchanged. Malformed input, permissions, cancellation, containment, and large-input regressions remain covered.

Verification

Local macOS, Node.js 24.18.0, npm 11.16.0:

Ten asynchronous suites passed five consecutive runs: 84 tests per run, 420 passing executions, retries disabled.
npm run test:coverage: 484 files passed, 4 skipped; 3,895 tests passed, 33 skipped.
Coverage: 85.10% statements, 75.72% branches, 88.04% functions, 86.78% lines; existing thresholds passed.
npm run check: 5/5 tasks passed; npm run docs:check and npm run format:check passed, including after the audit-document removal.
Relocated native NIB/storyboard cases and the exact-one-project discovery check passed.
npm run verify:interface-builder: passed against actual host ibtool and a source-built AppKit fixture.
CI follow-up 57e4a31f: the launcher suite passed another five consecutive local runs (40 passing executions); npm run check passed all five tasks after switching to readiness-marker publication.

Real Hopper/Ghidra installations were not exercised; their adapter tests use real subprocess/socket fixtures. Linux/Windows runtime verification belongs to hosted lanes.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/849 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/7/2026 **Status:** ✅ Merged **Merged:** 10/7/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `cleanup/test-suite-races` --- ### 📝 Commits (2) - [`116e510`](https://github.com/morluto/rea/commit/116e510bf5a8a904f8a2acf9cf4f9c8d24da1580) test: eliminate fixture races and prune redundant scaffolding - [`57e4a31`](https://github.com/morluto/rea/commit/57e4a31f968fbc79bb249a1528eaada0e8c972c2) fix(test): publish launcher capture readiness without renaming ### 📊 Changes **22 files changed** (+421 additions, -719 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/ci.yml` (+1 -1) ➖ `docs/test-suite-audit.md` (+0 -169) 📝 `docs/testing.md` (+0 -3) 📝 `src/domain/validationTaxonomy.test.ts` (+1 -36) 📝 `tests/boundary/browser/browserProcessStartup.test.ts` (+19 -20) 📝 `tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts` (+6 -5) 📝 `tests/boundary/browser/webSourceMapResponseCleanup.test.ts` (+10 -16) 📝 `tests/boundary/filesystem/interfaceBuilderBundle.test.ts` (+12 -21) 📝 `tests/boundary/filesystem/nibNativeHierarchy.test.ts` (+8 -5) 📝 `tests/boundary/filesystem/setupConfig.test.ts` (+22 -18) 📝 `tests/boundary/mcp/mcpDoctor.test.ts` (+21 -20) 📝 `tests/boundary/providers/ghidra/ghidraClient.test.ts` (+34 -17) 📝 `tests/boundary/providers/ghidra/ghidraLauncher.test.ts` (+54 -62) 📝 `tests/boundary/providers/hopper/hopperClient.cleanup.test.ts` (+13 -56) ➕ `tests/boundary/providers/hopper/hopperClient.fixture.ts` (+112 -0) 📝 `tests/boundary/providers/hopper/hopperClient.progress.test.ts` (+35 -79) 📝 `tests/boundary/providers/hopper/hopperClient.protocol.test.ts` (+35 -79) 📝 `tests/boundary/providers/hopper/hopperClient.restart.test.ts` (+6 -67) 📝 `tests/boundary/providers/hopper/hopperClient.startupFailures.test.ts` (+1 -41) 📝 `tests/fixtures/captureGhidraLaunch.mjs` (+5 -1) _...and 2 more files_ </details> ### 📄 Description ## Summary Address the test-suite audit findings with deterministic synchronization, shared fixture ownership, targeted pruning, and correct project placement. - Fix the CI-observed Ghidra capture race: publish a readiness marker only after closing the complete JSON capture, then validate it before asserting its fields. This also respects Windows private-runtime handles that restrict renames. - Replace Hopper/Ghidra sleep-based coordination with actual process/request acknowledgements and explicit response gates over test-only IPC. The late-reply case now delivers a real late reply; progress is asserted before releasing the terminal response. - Advance deadline timers only after real fixture startup, retain process/filesystem I/O, and remove the cancellation test's 500 ms host-speed assertion. - Await browser fixture termination and socket closure; consolidate five copied Hopper launchers and redundant temporary-root cleanup scaffolding. - Replace incidental frame-tree counts, option positions, and fixed request IDs with behavioral and correlation assertions. Report unavailable UID support as an explicit skip. - Remove the handwritten taxonomy-disjointness test; retain both empty-inventory failure-channel regressions. - Move Interface Builder filesystem/compiler tests from the service project to `tests/boundary/filesystem`, invoke ibtool asynchronously, and update the macOS CI command. All native hierarchy/storyboard assertions remain. - Delete the one-off `docs/test-suite-audit.md` artifact and its link from `docs/testing.md`; record decisions and verification here instead. ## Contract and boundary impact Test fixtures, test placement, CI routing, and documentation only. Production behavior, CLI/MCP schemas, catalog metadata, test retries, and coverage thresholds are unchanged. Malformed input, permissions, cancellation, containment, and large-input regressions remain covered. ## Verification Local macOS, Node.js **24.18.0**, npm **11.16.0**: - Ten asynchronous suites passed **five consecutive runs**: 84 tests per run, **420 passing executions**, retries disabled. - `npm run test:coverage`: **484 files passed, 4 skipped; 3,895 tests passed, 33 skipped**. - Coverage: **85.10% statements, 75.72% branches, 88.04% functions, 86.78% lines**; existing thresholds passed. - `npm run check`: **5/5 tasks passed**; `npm run docs:check` and `npm run format:check` passed, including after the audit-document removal. - Relocated native NIB/storyboard cases and the exact-one-project discovery check passed. - `npm run verify:interface-builder`: passed against actual host ibtool and a source-built AppKit fixture. - CI follow-up `57e4a31f`: the launcher suite passed another five consecutive local runs (40 passing executions); `npm run check` passed all five tasks after switching to readiness-marker publication. Real Hopper/Ghidra installations were not exercised; their adapter tests use real subprocess/socket fixtures. Linux/Windows runtime verification belongs to hosted lanes. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:16:46 +00:00