Bug/"flaky" test fix: Stop a stale runner list refetch from undoing runner updates
What does this MR do and why?
Context
This investigation started from https://gitlab.com/gitlab-org/quality/test-failure-issues/-/work_items/43855+,
where the pauses, resumes and deletes a runner shared example is flagged as
the rank 1 most pipeline-blocking flaky test. Three test-side fixes were previously
merged against it and none held.
It seems the spec test may not be flaky at all but a symptom of a real race condition in application code. Pausing a runner and then resuming it can leave the row showing "Paused" while the runner is actually running. There is no error and no spinner, and only a page reload clears it.
Currently, the runner list queries use a "cache and network" fetch policy with no
nextFetchPolicy, so Apollo re-runs them over the network on every cache
write. The pause mutation's own cache write therefore triggers a full list
refetch, issued immediately after the mutation response.
That refetch can read the server before the next mutation and land after it, overwriting the row with stale data. That row then contradicts the database until the page is reloaded.
This MR
Adds nextFetchPolicy: 'cache-first' to the three runner list queries. They
fetch over the network on the first load and whenever the variables change,
and serve later cache writes from the cache. Apollo restores the declared
policy on a variables change, so filtered and paginated results are still
fetched fresh, and refresh() still hits the network.
Nothing relied on the implicit refetch: deletion evicts the row from the
cache, and assigning, unassigning and the tab toggles call refresh().
Two consequences worth a reviewer's judgement:
- That refetch incidentally refreshed the whole list after any mutation, so other rows' statuses updated too. They no longer do until a reload or a filter change. These lists never polled, so this was a side effect of mutating rather than a freshness guarantee.
- With a pause-state filter active, a row you pause now stays in the list wearing a "Paused" badge instead of dropping out, while the tab count beside it updates.
Changelog: fixed
References
- https://gitlab.com/gitlab-org/quality/test-failure-issues/-/work_items/43855+
- Flaky test fix: reload page between pause and r... (!249222 - closed)
- Bug/"flaky" test fix: Avoid stale runner list a... (!249235 - merged)
Earlier test-side fixes, none of which held:
- Fix flaky runner pause/resume feature spec (!240917 - merged)
- Fix flaky runner pause/resume spec by waiting f... (!242256 - merged)
- Simplify flaky runner pause/resume spec assertions (!243022 - merged)
Screenshots or screen recordings
No visual change in normal use. The bug being fixed is itself visual: the row keeps its "Paused" badge after a resume succeeds.
How to set up and validate locally
-
Visit a group's runners page (
/groups/<group>/-/runners) with at least one runner. -
Click Pause, then click Resume as soon as the "Paused" badge appears.
-
On
masterthe row can keep the "Paused" badge whileCi::Runner#activeistrue. Delaying delivery of the list query's response so it lands after the resume mutation reproduces it every time. -
Run the specs:
bundle exec rspec spec/features/groups/runners spec/features/admin/runners \ spec/features/projects/runners yarn jest spec/frontend/ci/runner
MR acceptance checklist
Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.