HarperFast / HarperFast/harper

test:unit:all runs four suites in one 15-minute step, so a single hang costs every suite's result (v22 uses 10 of the 15 minutes)

Open
#2,587 0 comments 0 reactions 1 assignee Claimed by @kriszyp View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

## Summary

`test:unit:all` runs four suites serially inside a single step with `timeout-minutes: 15` (`.github/workflows/unit-test.yml:87-89`). The Node v22 leg is the slowest and already uses two thirds of that budget, so **any one hanging test consumes the rest and kills every suite's result, not just its own**.

Measured `Run tests` step durations, v22:

| where | duration | headroom under the 15 min step limit |
|---|---|---|
| `main` (run 34791149457) | **9m03s** | 5m57s |
| a feature branch (run 34823402619) | **10m06s** | 4m54s |
| same branch (run 34805886388) | **10m20s** | 4m40s |

Two runs on that branch hit the limit outright on the same night, with two different symptoms:

- [job 103848838606](https://github.com/HarperFast/harper/actions/runs/34802854042/job/103848838606) — `subscriptionPreviousCountScanBound` asserted on a log line and got `"JavaScript execution has taken too long and is not allowing proper event queue cycling"` instead: event-loop starvation, not a logic failure.
- [job 103851368764](https://github.com/HarperFast/harper/actions/runs/34803736178/job/103851368764) — `EntryHandler.test.js` hung for 14m38s and took the step to its timeout (evidence added to #2538, whose scope line currently says Linux CI is green on that file).

In both cases v24, v26 and Windows v24 passed the identical head, and `main`'s own v22 run was green.

## Why this is worth fixing rather than re-running

The headroom is what converts *one* flaky or hanging test into a **total loss of the unit gate**. When the step times out you get no summary at all, so you cannot tell which suites passed — and `apitests`, `resources` and `lmdb` never ran. That is the same structural cause as #2017 (`test:unit:all` silently drops later legs when an early leg fails): four suites chained behind one exit status and one clock.

It also degrades monotonically. Every test added to any of the four suites spends the shared budget; the branch measured above is ~1 minute slower than `main` purely from added coverage, and nothing surfaces that until a runner is slow enough to cross the line.

## Suggested fix shape

Any one of these removes the coupling; they are listed cheapest first.

1. **Raise `timeout-minutes` on the step** to something with real margin (30–35). One line, buys time, does not fix the blast radius.
2. **Split the four suites into their own steps**, each with its own timeout. A hang in `main` then still lets `resources` and `lmdb` report, and the step name says which suite died. Pairs naturally with #2017's fix.
3. **Split into matrix jobs** (`main` / `apitests` / `resources` / `lmdb`), which also parallelises and cuts wall-clock for every PR.

(2) is probably the right size: it addresses the blast radius, keeps one job, and makes the timeout attributable.

## Not in scope

The individual hangs — #2538 for `EntryHandler`, and whatever the `subscriptionPreviousCountScanBound` starvation turns out to be — are separate. This issue is about the step that turns either of them into a loss of the whole gate.

Found while maintaining #2498, which contributed ~1 minute of the v22 duration but is not the cause; `main` shows the same shape with more margin.

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.