HarperFast / HarperFast/harper-pro

decode-drop/poisoned-cursor cluster tests: one runs in no CI path, both leak the restarted Harper child

Open
#716 0 comments 0 reactions 1 assignee Claimed by @heskew View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.