test(perf): bcrypt cost 12 in tests makes the integration suite slow and timeout-flaky #242

New Issue

2026-08-30T12:10:35-05:00

bermudalamb commented

2026-08-30 12:10:35 -05:00

adminInventory.integration.test.ts failed in CI on 2026-08-30 at 01:12:

● admin customer reservations › a released item can be reserved again by someone else
  thrown: "Exceeded timeout of 20000 ms for a test."

Followed by a foreign key violation on customer_tokens, Key (customer_id)=(2) is not present in table "customers". That is the consequence, not the cause: the test hit its timeout, Jest moved on, beforeEach's TRUNCATE ... RESTART IDENTITY ran, and the still-in-flight registration then wrote a token for a customer that had just been deleted. Chasing the FK error leads nowhere.

Not a defect, and not the image work

The failing test contains zero image uploads — grep -c 'attach(' returns 0 — so #226's re-encoding is not involved despite adding real work to the upload path.

It is intermittent, not new. Run 651 earlier the same day passed integration 263/263 on effectively this code, and three consecutive local full runs passed.

What actually costs the time

bcryptjs at cost factor 12, hardcoded at four sites in src/routes/customers.ts, and applied identically in tests.

The integration suite performs roughly 35 registrations. bcryptjs is a pure-JS implementation, so it is far slower than a native one at the same cost — and cost 12 doubles the work of cost 11, and so on. The suite spends most of its wall clock hashing passwords that no test asserts anything about.

The failing test registers two customers in a single 20-second budget, on a runner that had six jobs queued and competing for CPU. It is the most exposed test rather than a broken one.

Why this is worth fixing rather than retrying

It makes the slowest CI job slower, on the single runner everything else queues behind. A full pass is around 21.5 minutes, and the queue has been six deep.
It makes timeouts a coin flip under load, so a red build becomes a judgement call rather than a signal — the same cost as #241, reached by a different route.
Retrying hides it. The failure is real, in the sense that the same load will produce it again.

Shape of a fix

Make the cost factor a constant read from the environment, defaulting to 12, and set something low (4–6) in the test setup only. Production behaviour must not change — the default stands when the variable is absent, so an environment that does not set it is unaffected.

Worth pairing with a check that the low cost cannot leak into a deployed environment, since a weakened password hash is a far worse outcome than a slow test suite. envValidation.ts is the natural place, and the existing DEMO_MODE strictness is the precedent: a setting that quietly degrades a security property should be a startup failure, not a warning.

scripts/bench-hash-latency.ts already exists to measure this and should be used to state the before and after rather than guessing at the improvement.

Related to #241 — both are about whether a red build means anything.

`adminInventory.integration.test.ts` failed in CI on 2026-08-30 at 01:12: ``` ● admin customer reservations › a released item can be reserved again by someone else thrown: "Exceeded timeout of 20000 ms for a test." ``` Followed by a foreign key violation on `customer_tokens`, `Key (customer_id)=(2) is not present in table "customers"`. That is the **consequence, not the cause**: the test hit its timeout, Jest moved on, `beforeEach`'s `TRUNCATE ... RESTART IDENTITY` ran, and the still-in-flight registration then wrote a token for a customer that had just been deleted. Chasing the FK error leads nowhere. ## Not a defect, and not the image work The failing test contains **zero** image uploads — `grep -c 'attach('` returns 0 — so #226's re-encoding is not involved despite adding real work to the upload path. It is intermittent, not new. Run 651 earlier the same day passed integration 263/263 on effectively this code, and three consecutive local full runs passed. ## What actually costs the time `bcryptjs` at cost factor **12**, hardcoded at four sites in `src/routes/customers.ts`, and applied identically in tests. The integration suite performs roughly **35 registrations**. `bcryptjs` is a pure-JS implementation, so it is far slower than a native one at the same cost — and cost 12 doubles the work of cost 11, and so on. The suite spends most of its wall clock hashing passwords that no test asserts anything about. The failing test registers **two** customers in a single 20-second budget, on a runner that had six jobs queued and competing for CPU. It is the most exposed test rather than a broken one. ## Why this is worth fixing rather than retrying - It makes the slowest CI job slower, on the single runner everything else queues behind. A full pass is around 21.5 minutes, and the queue has been six deep. - It makes timeouts a coin flip under load, so a red build becomes a judgement call rather than a signal — the same cost as #241, reached by a different route. - Retrying hides it. The failure is real, in the sense that the same load will produce it again. ## Shape of a fix Make the cost factor a constant read from the environment, defaulting to 12, and set something low (4–6) in the test setup only. Production behaviour must not change — the default stands when the variable is absent, so an environment that does not set it is unaffected. Worth pairing with a check that the low cost cannot leak into a deployed environment, since a weakened password hash is a far worse outcome than a slow test suite. `envValidation.ts` is the natural place, and the existing `DEMO_MODE` strictness is the precedent: a setting that quietly degrades a security property should be a startup failure, not a warning. `scripts/bench-hash-latency.ts` already exists to measure this and should be used to state the before and after rather than guessing at the improvement. Related to #241 — both are about whether a red build means anything.

bermudalamb referenced this issue from a commit

2026-08-30 12:29:31 -05:00

test(perf): stop hashing test passwords at production cost (#242)

bermudalamb referenced a pull request that will close this issue

2026-08-30 12:29:48 -05:00

test(perf): stop hashing test passwords at production cost (#242) #243

bermudalamb closed this issue

2026-08-30 13:32:15 -05:00

bermudalamb referenced this issue from a commit

2026-08-30 13:32:16 -05:00

Merge pull request 'test(perf): stop hashing test passwords at production cost (#242)' (#243) from fix/242-bcrypt-cost-in-tests into main

bermudalamb referenced this issue from a commit