Run more of the test suite in parallel #610

Closed
opened 2026-10-19 11:37:34 +02:00 by simon · 0 comments
Owner

Motivation

Every pipeline gate and every local just ci-dev runs the fast suite. On the 4-vCPU dev VM one run
takes about 4–5 minutes, and most of that time is serial. Two full runs on the #605 branch (2556 tests):
242.7 s total (37.5 s async, 205.2 s sync) and 246.5 s (36.2 s async, 210.2 s sync). Five runs in #598
measured 292–326 s, of which 249–274 s were sync. Under load (3 parallel pipeline issues) a run took
472 s (389 s sync). A pipeline run pays this many times: every /tdd session end, every review
iteration, and /finalize before and after the rewrite.

#533 removed the main reason many files were serial (FK deadlocks in concurrent create_member,
fixed with deferrable FKs), but the files were kept serial as a deferred decision.

Current state (main after #605)

  • 315 test files, 166 with async: false: 78 under test/mv/**, 87 under test/mv_web/**
    (51 of them in test/mv_web/live/member_live/).
  • 10 files carry the #533 note "re-flipping this … suite to async is a possible follow-up"; 9 are in
    test/mv_web/live/member_live/ (index_custom_fields_display, index_field_visibility,
    index_groups_{accessibility,display,filter,integration,performance,sorting},
    show_groups_display), plus test/mv_web/live/user_live/form_test.exs.
  • About 36 serial files touch global state (the Settings singleton, Logger.configure,
    Application.put_env, System.put_env). These must stay serial unless the state is isolated per
    test.
  • Largest serial files by test count: member_live/index_test.exs (75), member_overview/date_filter_test.exs (49),
    member_overview/payment_aging_test.exs (36), membership/member_policies_test.exs (31),
    global_settings_live_test.exs (25).
  • Test DB pool: System.schedulers_online() * 8 (config/test.exs:15); ExUnit runs with the default
    max_cases (2 × schedulers, test/test_helper.exs).
  • #605 already deleted 65 tests that could not fail or were duplicates; fewer serial tests are left to flip.

Known flake classes that more parallelism makes worse

Evidence log: .pipeline/followups/flaky-tests.md (Gitea #598). Two classes are still open and both
occur in tests that already run async, so flipping more files raises their rate:

  1. Postgres deadlocks between sandbox transactions (40P01). Four occurrences, all in a full
    suite run, never isolated:
    • member_export_controller_golden_test.exs setup (CustomField.create), twice (#600, #605)
    • group_test.exs "delete group cascades …" (create_member), #605
    • Error shape every time: Process A waits for ShareLock on transaction X; blocked by process B
      and vice versa. A ShareLock on a transaction is a wait for another uncommitted insert: a
      unique-index check (or an FK check) on a row the other test inserted in its sandbox
      transaction. Sandbox transactions never commit, so the waiting test blocks until the other test
      ends. If two tests insert two such rows in opposite order, Postgres detects the deadlock.
    • Hypothesis: async tests use fixed values under unique identities — CustomField unique_name
      / unique_slug, Group unique_slug, Member unique_email. The golden test creates groups
      "Board" / "Choir", fixed custom-field names and anna@example.com. name: "Board" also appears
      in 6 member_live test files, and anna@example.com in 6 other test files.
    • Confirm before fixing: right after a deadlock, the slot Postgres container log
      (docker logs mv-<issue>-db-1) has the query details the error hint points to. Or reproduce it
      with two async tests that insert the same two unique values in opposite order.
    • Fix direction: unique per-test values for every attribute under a unique identity, in fixtures
      and inline create_* calls (e.g. "Board-#{System.unique_integer([:positive])}"). Golden tests
      whose expected output contains the fixed value either derive the value or stay serial (with a
      one-line reason).
  2. Whole-page substring matches (refute html =~ name on the full rendered page). These match
    random CSRF/session tokens, or rows that other async tests create. Seen in
    index_groups_url_params_test.exs and import_live_test.exs (the token contained "BOM"). The
    test-hardening issue fixes this class suite-wide. A file flipped here must not contain it: scope
    the assertion to the element (has_element?, assert_flash/3, assert_field_error/3) in the
    same commit.

Resolved classes that stay rules for every flipped file: no capture_log assertions in async files,
because other processes' log lines get captured (#598, Class 2). No order assertions on reads
without an explicit sort (#598, Class 1).

Approach

  1. Deadlock class first. Confirm the cause (see above), then make unique-identity values unique
    per test in the files that already run async. Done when 5 seeded full-suite runs, one under CPU
    load, show no 40P01. Every later flip depends on it.
  2. Measure. Run just test-fast --slowest-modules 25 (3 runs, different seeds) and record the
    per-module time of the serial files. Choose candidates by time saved, not by count. If the
    measured gain for test/mv/** is small, stop after this step and record the numbers in the
    issue.
  3. Flip test/mv/** in that order, one file per commit. For each file, prove isolation per the
    async-safety checklist in docs/test-performance-optimization.md: green with several seeds and
    under CPU load, not one green run. Fix data assumptions that break under concurrency (global
    counts, "no other members exist", ordered reads without a sort, fixed unique values).
  4. Global-state files: for the expensive ones, move only the tests that need the global state
    into a small serial module and make the rest async. Isolate state per test where that is cheap
    (e.g. pass settings explicitly instead of mutating the singleton).
  5. Leave the rest serial and record the reason in a one-line comment at use … async: false, so
    the next reader does not have to rediscover it.
  6. Write the rules into CODE_GUIDELINES.md §4 (next to the async/sandbox section): unique
    values for unique identities in tests, the reason comment on every async: false, and no
    capture_log in async files.

Coordination with other issues

  • #605 (dead-code issue) is done; this issue was meant to run after it.
  • Test hardening (the issue from issue-test-hardening-and-guidelines.md): it touches the same
    test/mv/** files (policies, fee and custom-field domain). Run the two one after the other, not in
    parallel. This issue goes first, so the hardening runs on the faster suite.
  • Flip test/mv/** first. The 51 serial files in test/mv_web/live/member_live/ change with #602
    (buttons/theme), U7 (search), U10 (list toolbars) and #549 (saved views). Flip them after those
    issues, or as part of them; this issue does not touch test/mv_web/live/**.
  • #540 (split global_settings_live.ex): make the settings state isolatable per section there, so
    its tests can run async.
  • A shorter suite makes #98 (pre-commit checks) practical.

Acceptance criteria

  • The 40P01 cause is confirmed (container log or repro), recorded in .pipeline/followups/flaky-tests.md,
    and fixed. Five seeded full-suite runs, one under CPU load, show no deadlock.
  • The PR description has before/after numbers for the fast suite: total, async and sync time, 3 runs
    each, same host, seeds recorded.
  • Every flipped file passed at least 5 seeded full-suite runs, one of them under CPU load. The runs are
    listed in the evidence log, following the .pipeline/followups/flaky-tests.md pattern.
  • No file is flipped without a measured gain. Every remaining async: false file has a one-line reason.
  • Flipped files contain no whole-page substring assertions, no capture_log assertions, and no fixed
    values under a unique identity.
  • CODE_GUIDELINES.md §4 has the rules from step 6.
  • just ci-dev and just typecheck are green.

Risks

  • New flakes of exactly the kinds #598 removed. Mitigation: the checks in
    docs/test-performance-optimization.md, plus seeded runs under load.
  • The deadlock hypothesis may be incomplete, for example an FK check against a row another test is
    deleting. The container log decides; do not flip files before step 1 is green.
  • The gain is bounded by 4 vCPUs and the DB. Estimate (not measured): about 4 → 3 minutes if half of
    the serial time becomes parallel. Step 2 decides whether the work is worth it.

Out of scope

  • CI-level partitioning (MIX_TEST_PARTITION across machines); it does not help on a single VM.
  • Speeding up individual slow tests (separate from the async/sync split).
  • Flipping test/mv_web/live/** (see Coordination).
## Motivation Every pipeline gate and every local `just ci-dev` runs the fast suite. On the 4-vCPU dev VM one run takes about 4–5 minutes, and most of that time is serial. Two full runs on the #605 branch (2556 tests): 242.7 s total (37.5 s async, 205.2 s sync) and 246.5 s (36.2 s async, 210.2 s sync). Five runs in #598 measured 292–326 s, of which 249–274 s were sync. Under load (3 parallel pipeline issues) a run took 472 s (389 s sync). A pipeline run pays this many times: every `/tdd` session end, every review iteration, and `/finalize` before and after the rewrite. #533 removed the main reason many files were serial (FK deadlocks in concurrent `create_member`, fixed with deferrable FKs), but the files were kept serial as a deferred decision. ## Current state (main after #605) - 315 test files, 166 with `async: false`: 78 under `test/mv/**`, 87 under `test/mv_web/**` (51 of them in `test/mv_web/live/member_live/`). - 10 files carry the #533 note "re-flipping this … suite to async is a possible follow-up"; 9 are in `test/mv_web/live/member_live/` (`index_custom_fields_display`, `index_field_visibility`, `index_groups_{accessibility,display,filter,integration,performance,sorting}`, `show_groups_display`), plus `test/mv_web/live/user_live/form_test.exs`. - About 36 serial files touch global state (the Settings singleton, `Logger.configure`, `Application.put_env`, `System.put_env`). These must stay serial unless the state is isolated per test. - Largest serial files by test count: `member_live/index_test.exs` (75), `member_overview/date_filter_test.exs` (49), `member_overview/payment_aging_test.exs` (36), `membership/member_policies_test.exs` (31), `global_settings_live_test.exs` (25). - Test DB pool: `System.schedulers_online() * 8` (`config/test.exs:15`); ExUnit runs with the default `max_cases` (2 × schedulers, `test/test_helper.exs`). - #605 already deleted 65 tests that could not fail or were duplicates; fewer serial tests are left to flip. ## Known flake classes that more parallelism makes worse Evidence log: `.pipeline/followups/flaky-tests.md` (Gitea #598). Two classes are still open and both occur in tests that **already** run async, so flipping more files raises their rate: 1. **Postgres deadlocks between sandbox transactions (40P01).** Four occurrences, all in a full suite run, never isolated: - `member_export_controller_golden_test.exs` setup (`CustomField.create`), twice (#600, #605) - `group_test.exs` "delete group cascades …" (`create_member`), #605 - Error shape every time: `Process A waits for ShareLock on transaction X; blocked by process B` and vice versa. A ShareLock on a *transaction* is a wait for another uncommitted insert: a unique-index check (or an FK check) on a row the other test inserted in its sandbox transaction. Sandbox transactions never commit, so the waiting test blocks until the other test ends. If two tests insert two such rows in opposite order, Postgres detects the deadlock. - Hypothesis: async tests use fixed values under unique identities — `CustomField` `unique_name` / `unique_slug`, `Group` `unique_slug`, `Member` `unique_email`. The golden test creates groups "Board" / "Choir", fixed custom-field names and `anna@example.com`. `name: "Board"` also appears in 6 member_live test files, and `anna@example.com` in 6 other test files. - Confirm before fixing: right after a deadlock, the slot Postgres container log (`docker logs mv-<issue>-db-1`) has the query details the error hint points to. Or reproduce it with two async tests that insert the same two unique values in opposite order. - Fix direction: unique per-test values for every attribute under a unique identity, in fixtures and inline `create_*` calls (e.g. `"Board-#{System.unique_integer([:positive])}"`). Golden tests whose expected output contains the fixed value either derive the value or stay serial (with a one-line reason). 2. **Whole-page substring matches** (`refute html =~ name` on the full rendered page). These match random CSRF/session tokens, or rows that other async tests create. Seen in `index_groups_url_params_test.exs` and `import_live_test.exs` (the token contained "BOM"). The test-hardening issue fixes this class suite-wide. A file flipped here must not contain it: scope the assertion to the element (`has_element?`, `assert_flash/3`, `assert_field_error/3`) in the same commit. Resolved classes that stay rules for every flipped file: no `capture_log` assertions in async files, because other processes' log lines get captured (#598, Class 2). No order assertions on reads without an explicit sort (#598, Class 1). ## Approach 1. **Deadlock class first.** Confirm the cause (see above), then make unique-identity values unique per test in the files that already run async. Done when 5 seeded full-suite runs, one under CPU load, show no 40P01. Every later flip depends on it. 2. **Measure.** Run `just test-fast --slowest-modules 25` (3 runs, different seeds) and record the per-module time of the serial files. Choose candidates by time saved, not by count. If the measured gain for `test/mv/**` is small, stop after this step and record the numbers in the issue. 3. **Flip `test/mv/**`** in that order, one file per commit. For each file, prove isolation per the async-safety checklist in `docs/test-performance-optimization.md`: green with several seeds and under CPU load, not one green run. Fix data assumptions that break under concurrency (global counts, "no other members exist", ordered reads without a sort, fixed unique values). 4. **Global-state files:** for the expensive ones, move only the tests that need the global state into a small serial module and make the rest async. Isolate state per test where that is cheap (e.g. pass settings explicitly instead of mutating the singleton). 5. **Leave the rest serial** and record the reason in a one-line comment at `use … async: false`, so the next reader does not have to rediscover it. 6. **Write the rules into `CODE_GUIDELINES.md` §4** (next to the async/sandbox section): unique values for unique identities in tests, the reason comment on every `async: false`, and no `capture_log` in async files. ## Coordination with other issues - #605 (dead-code issue) is done; this issue was meant to run after it. - **Test hardening** (the issue from `issue-test-hardening-and-guidelines.md`): it touches the same `test/mv/**` files (policies, fee and custom-field domain). Run the two one after the other, not in parallel. This issue goes first, so the hardening runs on the faster suite. - Flip `test/mv/**` first. The 51 serial files in `test/mv_web/live/member_live/` change with #602 (buttons/theme), U7 (search), U10 (list toolbars) and #549 (saved views). Flip them after those issues, or as part of them; this issue does not touch `test/mv_web/live/**`. - #540 (split `global_settings_live.ex`): make the settings state isolatable per section there, so its tests can run async. - A shorter suite makes #98 (pre-commit checks) practical. ## Acceptance criteria - The 40P01 cause is confirmed (container log or repro), recorded in `.pipeline/followups/flaky-tests.md`, and fixed. Five seeded full-suite runs, one under CPU load, show no deadlock. - The PR description has before/after numbers for the fast suite: total, async and sync time, 3 runs each, same host, seeds recorded. - Every flipped file passed at least 5 seeded full-suite runs, one of them under CPU load. The runs are listed in the evidence log, following the `.pipeline/followups/flaky-tests.md` pattern. - No file is flipped without a measured gain. Every remaining `async: false` file has a one-line reason. - Flipped files contain no whole-page substring assertions, no `capture_log` assertions, and no fixed values under a unique identity. - `CODE_GUIDELINES.md` §4 has the rules from step 6. - `just ci-dev` and `just typecheck` are green. ## Risks - New flakes of exactly the kinds #598 removed. Mitigation: the checks in `docs/test-performance-optimization.md`, plus seeded runs under load. - The deadlock hypothesis may be incomplete, for example an FK check against a row another test is deleting. The container log decides; do not flip files before step 1 is green. - The gain is bounded by 4 vCPUs and the DB. Estimate (not measured): about 4 → 3 minutes if half of the serial time becomes parallel. Step 2 decides whether the work is worth it. ## Out of scope - CI-level partitioning (`MIX_TEST_PARTITION` across machines); it does not help on a single VM. - Speeding up individual slow tests (separate from the async/sync split). - Flipping `test/mv_web/live/**` (see Coordination).
simon closed this issue 2026-10-19 21:13:44 +02:00
Sign in to join this conversation.
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
local-it/mitgliederverwaltung#610
No description provided.