HarperFast / HarperFast/harper-pro
decode-drop/poisoned-cursor cluster tests: one runs in no CI path, both leak the restarted Harper child
- Dominant language
- JavaScript
- Stars
- 3
- Forks
- 0
- Avg merge
- 1d 21h
- Merged PRs (30d)
- 80
Description
Surfaced by @kriszyp's post-approval review on #691 (anchors failed, so the findings live in the review body). Companion to #715.
## 1. `poisonedCopyCursorDataLoss.test.mjs` runs in no CI path (verified)
The stress matrix (`.github/workflows/stress-tests.yaml:136`) includes `integrationTests/cluster/decodeDropRecovery.test.mjs` but not `integrationTests/cluster/poisonedCopyCursorDataLoss.test.mjs`. That suite is gated on `HARPER_RUN_STRESS_TESTS=1`, which normal cluster CI does not set — so the 279-line characterization test (and the production injection hook added for it) never executes anywhere. Add it to the stress matrix, or drop the stress gate if its runtime fits regular cluster CI.
## 2. Restart/teardown leak pattern in both cluster tests (per the review; confirm while fixing)
In `decodeDropRecovery.test.mjs:120` (and the same pattern at `poisonedCopyCursorDataLoss.test.mjs:158-194`):
- `deploy_component` with `restart: true` responds before the old process exits, so the fixed sleep can race the outgoing process.
- The replacement Harper process is detached from the child handle the integration harness retains, so `teardownHarper` cannot stop it — the stress job can fail through the leaked-child backstop even when assertions pass.
- Cleanup calls `teardownHarper(ctx.nodeA)` instead of the required `{ harper: ctx.nodeA }` shape.
Suggested fix per the review: deploy without the implicit restart, use `restartNode()` plus readiness checks, and call `stopNodeProcess()` before `teardownHarper({ harper: node })`.
Contributor guide
Assessment
This issue has not been assessed yet.