HarperFast / HarperFast/harper
Migrate unitTests/apiTests to the standard integration testing framework
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
## Summary
`unitTests/apiTests` is an integration test suite wearing a unit test's clothes. It boots Harper **in the mocha process**, monkeypatches auth in-memory, and isolates itself with in-process `env.setProperty()` calls that worker threads never see. It should move to `integrationTests/` and the standard `@harperfast/integration-testing` framework, which already provides exactly the isolation the harness is hand-rolling.
## Why now
**1. It is actively hostile to concurrent runs.** The suite's entry point is:
```
test:unit:apitests = node ./dist/bin/harper.js stop && mocha "unitTests/apiTests/**/*-test.mjs"
```
It **stops the shared ambient install** before running. With multiple agents or developers working on one box, starting apitests kills whatever Harper anyone else has running. No amount of loopback-address isolation fixes this — it's a global side effect baked into the entry point, and it exists because the suite boots Harper in-process and needs the ambient instance out of the way.
**2. The in-process override mechanism doesn't survive thread boundaries.** `setProperty()` deliberately never writes the config file, and `flatConfigObj` is module-scoped — a worker thread is its own V8 realm and re-reads `harper-config.yaml` from disk. So a worker silently falls back to whatever is actually installed. This is not a race; it is a permanent divergence. CI is green only because its fresh `DEFAULTS_MODE=dev` / `NODE_HOSTNAME=localhost` install happens to be shaped like what the harness overrides to. On a dev install with `securePort` set or a non-`localhost` hostname, the suite fails (181/10 observed).
harper#2024 fixes that divergence by propagating overrides through `workerData`. That works, but it puts test-only machinery on the production worker-spawn path — a reasonable stopgap, not an end state. Migrating removes the need for it entirely.
**3. It is already half-migrated.** `setupTestApp.mjs` already imports `getNextAvailableLoopbackAddress` / `releaseAllLoopbackAddressesForCurrentProcess` from `@harperfast/integration-testing`, and `integrationTests/apiTests/` already exists with `rest`, `graphql`, `authentication`, `token-auth`, `headers`, and `transactions`. This is finishing a migration in progress, not new architecture.
## What has to change
The tests themselves are mostly already black-box — they talk to the server over HTTP/WS/MQTT via axios and real clients. The work is concentrated in `setupTestApp.mjs`:
| Current (in-process) | Target |
| --- | --- |
| `startHTTPThreads()` called directly | `startHarper()` child process with a temp install dir |
| `bypassAuth()` + `bypassAuthMQTT()` + `server.getUser` monkeypatch | Real users/roles created via the operations API at setup |
| `getDatabases()` + `setupTestDBPath()` system-database preservation dance | Disappears — it exists only because `rootPath` is swapped under a live process |
| `analytics.setAnalyticsEnabled(false)` | Needs a config-level equivalent — `setAnalyticsEnabled()` is currently an in-process setter with no obvious config key; may require adding one |
| `env.setProperty()` port/address/socket overrides | Real config in the temp install's `harper-config.yaml` |
| `RUN_HDB_APP` pointing at `unitTests/testApp` | Component deployed into the temp install |
| `_UNREF_SERVER`, `_DISABLE_NATS`, `SCHEMAS_DATA_PATH` env hacks | Not needed with a real child process |
| `addThreads()` (dynamic mid-run thread addition) | **No direct equivalent** — needs a configured thread count and restart, or the test is restructured |
`addThreads()` (used only by `multi-threaded-test.mjs`) is the one capability with no clean child-process analogue and should be decided explicitly rather than ported.
## Inventory
| File | Lines | Notes |
| --- | --- | --- |
| `mqtt-test.mjs` | 1396 | Bulk of the work; 31 cases. Includes mTLS cert-selection logic recently touched by #2024 |
| `graphql-querying-test.mjs` | 712 | Table-driven; few `it()` blocks, wide coverage |
| `basicREST-test.mjs` | 650 | 57 cases |
| `RESTProperties-test.mjs` | 401 | 21 cases |
| `setupTestApp.mjs` | 189 | The harness — rewritten, not ported |
| `cache-test.mjs` | 128 | |
| `ws-json-work.mjs` | 125 | |
| `ws-test.mjs` | 118 | |
| `multi-threaded-test.mjs` | 98 | Depends on `addThreads()` |
| `undefinedIdInUrl-test.mjs` | 90 | |
| `RBAC-test.mjs` | 47 | Overlaps `integrationTests/apiTests/authentication.test.mjs` — check before porting |
| `fastifyRoutes-test.mjs` | 34 | |
~4,000 lines total. This is a genuine port, not delete-and-adopt: the unit apitests are deeper than their existing integration counterparts (REST 57 cases vs 19 in `rest.test.mjs`; MQTT 31 vs 14 in `mqtt.test.ts`). A coverage-overlap audit should run first so we delete rather than duplicate where the integration suite already covers a case.
## Constraints to preserve
- `test:unit:lmdb` runs apitests under `HARPER_STORAGE_ENGINE=lmdb`. That matrix dimension has to survive the move.
- `test:unit:all` and the CI workflow both reference `test:unit:apitests`; the migration has to land the CI wiring in the same change or the coverage silently drops.
## Definition of done
- [ ] Coverage-overlap audit against `integrationTests/apiTests/` and `integrationTests/mqtt/`
- [ ] All non-duplicate cases ported to `integrationTests/`, running under `harper-integration-test-run`
- [ ] `unitTests/apiTests/` removed, along with `test:unit:apitests` and the `harper.js stop` prelude
- [ ] LMDB matrix coverage preserved
- [ ] CI workflow updated; no net coverage loss
- [ ] **The `configOverrides` worker-propagation mechanism from #2024 removed** (`environmentManager.ts` override tracking + `manageThreads.js` workerData provider). The `updateConfigObject()` nested-tree mirror from that PR **stays** — it fixes a genuine main-thread bug on the install path, independent of the tests.
## Related
- harper#2024 — the stopgap that makes the current harness work on non-CI-shaped installs
- Known gap noted in #2024, unaffected by it and unaffected by this migration: Plugin API components (`OptionsWatcher`) re-parse `harper-config.yaml` off disk and never consult `getConfigObj()`/`flatConfigObj`, so `env.setProperty()` overrides are invisible to them. Worth its own issue.
Contributor guide
Research direction
Start with the coverage-overlap audit between unitTests/apiTests and integrationTests/apiTests plus integrationTests/mqtt. Read setupTestApp.mjs and the listed test files, then inspect the harper-integration-test-run entry point and CI references to test:unit:apitests. Done means non-duplicate coverage runs under the standard framework, LMDB and CI coverage remain, and the old harness and worker override mechanism are removed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- backend, ci-cd, testing-qa
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100