Created Started Cancelled
Declare one sqitch test per change instead of one for the whole plan Fixes the sydtest side of https://github.com/NorfairKing/nix-ci/issues/772. ## The bug `nix flake check` on nix-ci fails intermittently with a `<<timeout>>` on ``` sqitch sanity checks.round-trips and (unless grandfathered) is idempotent for every change in sqitch.plan ``` naming a different change each time (`coverage-date-interval`, `add-suite-gitlab`, ...). The same suite passes standalone in about 25 seconds. `perChangeIt` was a single `it` that looped over every change in `sqitch.plan`. For each change it deploys, reverts, redeploys and compares the schema, shelling out to sqitch (Perl, with its own psql connections) each time. With 28 changes that is roughly a hundred process spawns and several hundred database round-trips, all inside one fixed 60 second per-test budget. Under `nix flake check` the machine is heavily oversubscribed, and process spawns and connection setup degrade worst under exactly that, so roughly a 2x slowdown blows the budget. A different change named each time is the giveaway: it is wherever the clock ran out, not a slow migration. There is a slow-moving version of the same problem with no load involved: every migration added makes that one test longer while its budget stays at 60 seconds. ## The test that reproduces it `sydtest-sqitch-postgres/test/Test/Syd/Sqitch/PostgresqlSpec.hs` builds the test forest for `test_resources/toy-sqitch-ok` with `execTestDefM` and asserts the rendered test ids. It needs no postgres and no sqitch run, it just inspects what the combinator declares: ``` sqitch sanity checks.every change round-trips and (unless grandfathered) is idempotent.init sqitch sanity checks.every change round-trips and (unless grandfathered) is idempotent.add-color@v1 sqitch sanity checks.every change round-trips and (unless grandfathered) is idempotent.add-color sqitch sanity checks.the whole plan deploys, reverts, and redeploys to the same schema ``` It is committed on its own (d3adb60) before the fix, where it fails: the forest held a single test named `round-trips and (unless grandfathered) is idempotent for every change in sqitch.plan`. I did not write a test for the timeout itself. Any such test would be a wall-clock race, and the thing that actually went wrong is structural: one budget covering work that grows without bound. ## The fix `sqitchPostgresqlSpec` reads the plan at spec-definition time (via `runIO`) and declares one test per change. Each one runs in a second or two against its own 60 second budget, so a large slowdown stays well inside it and the budget stops shrinking as the plan grows. The failing change is now a test name instead of whatever happened to be in flight. The per-change tests still walk the plan in order against a single database, each deploying on top of what the previous one left behind. That is deliberate and unchanged: starting every change from a clean database would hide interactions between migrations, and would be quadratic. The shared state is why the group is `setupAroundAll` + `sequential` + `doNotRandomiseExecutionOrder` (sydtest randomises execution order by default). `runSqitchPerChangeChecks` still does the whole walk in one `IO` action, for the negative tests that wrap it in `expectFailing`. It and the new per-change tests share `checkStep`, so there is one copy of the round-trip and idempotence logic. Version bumped to 0.2.0.0 with a changelog entry: test names under `sqitch sanity checks` change, which matters to anyone filtering on them. ## Things to check manually - **The name I picked for the group**: `every change round-trips and (unless grandfathered) is idempotent`, with the change as the test name under it. Previously the change was a `context`, now it is the description. - **Shared database across the per-change tests.** A failure in one change leaves the next tests running against whatever state sqitch left. The first failure is still the informative one, but the ones after it may be noise. The alternative (a clean database per change) is quadratic, so I kept the shared walk. - **nix-ci's own run.** Worth re-running `nix flake check` there against this branch to confirm the leader-data-gen flakiness is gone, including without `--max-jobs 4`. - Both affected suites pass locally and `nix flake check` passes here.