Flaky: 100-row paged-list tests time out under CI load (ExpensesPage, CustomersPage) #557

Activity

1. mforce

   added

   bugSomething isn't workingSomething isn't working

   area:frontendReact/Vite web clientReact/Vite web client

   on Aug 18, 2026
2. mforce

   added

   severity:p4Defect: tooling, CI noise or test gap; no user impactDefect: tooling, CI noise or test gap; no user impact

   on Aug 31, 2026
3. mforce commented on Aug 31, 2026

   

   mforce

   on Aug 31, 2026

   OwnerAuthor

   More actions

   Duplicate #563 filed the same flake independently (observed on PR #561, a backend-only PR touching no web/ files). Closing that one in favour of this issue; its root-cause analysis is worth carrying here rather than losing:

   The test builds a 100-row first page, then does two findByRole("row", { name: /…/ }) lookups against it. A role query with a name filter walks the whole accessibility tree per lookup, so the cost scales with the row count — which is why the 5s default Vitest timeout is tight enough to flip under CI load.

   That matches the fix direction already recorded here: wait on the concrete post-load state (appended rows, or the "load more" control re-enabling) rather than racing the async load. Narrowing the first page, or scoping the row lookups, addresses the tree-walk cost directly.

   Both reports are on commits that changed no web/ files (#561, and 4148f9b on main), so this is pre-existing flake, not a regression from either.
4. mforce commented on Sep 1, 2026

   

   mforce

   on Sep 1, 2026

   OwnerAuthor

   More actions

   Third occurrence, different file, same shape — so the scope of this issue is wider than ExpensesPage.

   Observed on run 33466025914 (PR #635, chore/track-specify-tooling — a docs/tooling-only branch touching no web/ source, so again pre-existing flake, not a regression):

   FAIL  src/routes/CustomersPage.test.tsx > CustomersPage paging (#511) > keeps the loaded window after a create instead of snapping back to page one
   Error: Test timed out in 5000ms.
    ❯ src/routes/CustomersPage.test.tsx:857:3
   
    Test Files  1 failed | 87 passed (88)
         Tests  1 failed | 2051 passed (2052)
   

   CustomersPage.test.tsx as a file took 22.1s for 45 tests in that run — the slowest file in the suite.

   Same root cause

   The failing test is the exact pattern the #563 analysis carried here: build customerPage(100), then repeatedly findByRole("row", { name: /…/ }) against it. This one does four such name-filtered role queries (p customer 000, Zulu Farm, then Zulu Farm again after the create refresh), each of which walks the whole 100-row accessibility tree, plus two full 100-row re-reads triggered by the create refresh. Under a loaded runner that overruns 5s.

   Corroborating evidence that the tree-walk cost is the driver, not the async race: the next test in the same describe block (refreshes every loaded page after an edit…) does the same 100-row paging dance but resolves rows through a local findCustomerRow helper built on findByText(name, { selector: "td" }) — a cheap text query — and it did not flake. The cheap-lookup workaround is already present one test below the one that failed.

   Consequence for the fix

   The fix recorded in the description ("wait on the concrete post-load state… narrow the first page, or scope the row lookups") is still right, but it must be applied as a pattern sweep across the paged-list tests, not a one-test patch on ExpensesPage — otherwise the next 100-row findByRole("row", { name }) site flakes next. Suggest the fix pass:

   1. Grep the web/ tests for name-filtered ByRole("row", …) lookups against 100-row fixtures and convert them to a scoped/text lookup (the findCustomerRow helper is the working reference).
   2. Narrow the fixture where the test does not actually need 100 rows — most only need "a full page" as a paging signal, which the page-size constant can express.
   3. Keep the global testTimeout alone.

   Suggest retitling this issue to something like Flaky: 100-row paged-list tests time out under CI load (ExpensesPage, CustomersPage) so the next occurrence lands here instead of being filed as a third duplicate.
5. mforce commented on Sep 1, 2026

   

   mforce

   on Sep 1, 2026

   OwnerAuthor

   More actions

   Triage: root cause confirmed, measured, and scoped

   Investigated the CustomersPage occurrence from run 33466025914. The comment above had the right instinct; here is the measurement behind it, plus one correction to the proposed fix.

   It is query cost, not an async race

   ByRole("row", { name }) makes Testing Library compute the accessible name of every row in the document (dom-accessibility-api walks each row's whole subtree and concatenates its text). Against a 100-row table that is ~600 node walks per attempt — and findBy* re-runs the entire query on every DOM mutation until it matches, so during a paged load the cost multiplies by the number of re-renders.

   The awaited state in the failing test is correct. Nothing is racing. The test simply does too much work to finish inside the 5s default on a loaded runner.

   Measurements (local, npx vitest run src/routes/CustomersPage.test.tsx)

   test	lookup style	duration
   keeps the loaded window after a create… (the flaky one)	4× findByRole("row", { name })	616 ms
   refreshes every loaded page after an edit… (next test, same paging dance)	findByText(name, { selector: "td" })	71 ms

   Swapping only the three row lookups in the failing test to the cheap text form: 616 ms → 264 ms (2.3×), all 45 tests still green.

   Scaling check: that file runs in 3.5 s locally and took 22.1 s on the CI runner — a ~6.3× slowdown. 616 ms × 6.3 ≈ 3.9 s, which is inside the same order as the 5000 ms budget. That is exactly the margin that intermittently disappears, and it explains why a docs-only branch trips it.

   Correction: shrinking the fixture is not available

   The comment's fix #2 ("narrow the fixture where the test does not need 100 rows") cannot work as written. usePagedList derives hasMore from page.length === pageSize, and each screen's page size is a module-private constant (CUSTOMER_PAGE = 100 in CustomersPage, PAGE in ExpensesPage/AuditPage). A fixture below the page size withdraws the pager, which is the thing these tests assert. Exporting the constant would remove the magic number but not a single row. The fix has to be the cheap lookup.

   Blast radius

   Name-filtered row queries in the test files that build page-size fixtures:

   file	name-filtered ByRole("row") queries
   CustomersPage.test.tsx	39
   InventoryPage.test.tsx	28
   ExpensesPage.test.tsx	17
   AuditPage.test.tsx	16

   Only the ones executed while a page-size fixture is mounted are expensive, but the pattern is one edit away from becoming expensive in any of them.

   TDD Fix Plan

   1. RED — Write a test for a shared findRowByCellText(name) test helper: given a rendered table, it resolves the <tr> whose <td> carries the exact text, and fails with a message naming the missing text when it does not appear.
      GREEN — Add the helper alongside the other shared test utilities. It is the findCustomerRow local helper, promoted.

   2. RED — Add a guard test that scans the paged-list test files (the ones that build a page-size fixture) and fails while any name-filtered row-role query remains in them. It fails immediately, on four files.
      GREEN — Convert the CustomersPage paging tests to the helper. Guard still red, one file down.

   3. RED — Guard is still red on ExpensesPage.
      GREEN — Convert the ExpensesPage pagination tests, including the originally reported 'load more' test.

   4. RED — Guard is still red on AuditPage and InventoryPage.
      GREEN — Convert their paged-list tests. Guard goes green.

   5. RED — Pin the observable outcome: with the conversions in place, the full web suite passes with the per-test timeout left at its default while the paged-list files run under artificial load (parallel workers plus a CPU load generator), repeated. Record before/after per-test durations for the four converted tests.
      GREEN — Nothing new; this cycle exists to prove the flake is gone rather than merely rarer.

   REFACTOR — Export each screen's page-size constant and build the fixtures from it, so a fixture length and the hasMore condition it depends on cannot silently drift apart.

   Acceptance Criteria

   • A shared row-by-cell-text helper exists and is unit-tested.
   • No name-filtered row-role query survives in any test file that mounts a page-size fixture, enforced by a guard test rather than by review.
   • The four previously slow paging tests each run in a small fraction of their prior duration; the numbers are recorded in the PR.
   • The global testTimeout is unchanged.
   • The web suite is green across repeated runs on a deliberately loaded machine.
   • All new tests pass; existing tests still pass.

   Retitling this issue per the comment above so the next occurrence lands here.
6. mforce

   changed the title [-]Flaky: ExpensesPage pagination 'load more' test times out under CI load[/-] [+]Flaky: 100-row paged-list tests time out under CI load (ExpensesPage, CustomersPage)[/+] on Sep 1, 2026
7. mforce

   added 5 commits that reference this issue on Sep 1, 2026

   test(web): name it.each blocks correctly in the paged-row lint (#557)

   ...

   7024182

   test(web): close two detection gaps in the paged-row lint (#557)

   ...

   e8ad917

   test(web): match shorthand { name } in the paged-row lint (#557)

   ...

   3f5bfe7

   test(web): stop the paged-row lint matching a quoted "name" value (#557)

   ...

   7314ad8

   test(web): resolve paged-list rows by cell text, not accessible name (#…

   ...

   66ecb94
8. mforce commented on Sep 2, 2026

   

   mforce

   on Sep 2, 2026

   Last edited by mforce

   OwnerAuthor

   More actions

   Fixed by #636 (merged 2026-09-01). The PR footer read Refs #557 rather than Fixes, so this never auto-closed.

   The root cause turned out not to be the async load race this report guessed. ByRole("row", { name }) makes Testing Library compute the accessible name of every row in the document — a recursive subtree walk per row — and findBy* re-runs the whole query on every DOM mutation. On a 100-row fixture re-rendered through a paged load, that overruns the 5s default under a loaded runner.

   What shipped:

   • web/src/test/rows.ts — findRowByCellText / getRowByCellText: one indexed text query plus closest("tr"), promoted from the hand-rolled helper that sat next to the flaky test and had never flaked.
   • web/src/test/pagedRowLookups.test.ts — a lint that fails when a paged-list test reaches for the expensive lookup shape.
   • ExpensesPage.test.tsx and CustomersPage.test.tsx converted; testTimeout was left untouched, per this issue's "do NOT just bump the global timeout".

   Duplicate #563 was already closed into this one. The acknowledged limit — the guard is a text scan, so it misses a fixture built behind a beforeEach helper or a page size that stops being 100 — is tracked as #637.
9. mforce

   closed this as completedon Sep 2, 2026
10. mforce

    added a commit that references this issue on Sep 10, 2026

    test(web): raise the paging suites' per-test budget, not the run's (#750

    ...

    ea84801