Reorder Capybara assertions to confirm state before absence

Pedro Pombeirorequested to merge
pedropombeiro/reorder-capybara-assertions-flaky into master

What does this MR do and why?

Fixes a class of flaky :js feature specs that intermittently time out (30s) on negative Capybara matchers that run before a positive matcher confirms the post-action state.

The anti-pattern

After an action that triggers an asynchronous UI re-render (click_button, click_link, etc.), these specs asserted the absence of the old state before confirming the new state:

click_button 'Resume'
expect(page).not_to have_button 'Resume'   # negative FIRST — waits the full 30s
expect(page).to have_button 'Pause'        # positive LAST

While the component is mid-transition, the old element is still in the DOM, so the negative matcher polls it for the entire Capybara timeout before concluding it's gone — a 30s flake when the re-render is slightly slow.

The fix

Reorder each affected block so a positive matcher confirms the new state first, then the negative absence checks run against an already-settled page:

click_button 'Resume'
expect(page).to have_button 'Pause'        # positive first — waits for re-render
expect(page).not_to have_button 'Resume'   # negative now resolves instantly

This follows the testing guide: confirm the page is loaded with a positive matcher before checking for element absence (best practices).

Pure assertion reordering — no expectations were added or removed (43 insertions / 43 deletions across 25 files). The same fix was previously applied to the runner specs in !240917 (merged).

Scope

25 feature specs across CE and EE, covering award emoji toggles, label subscription, admin background migrations, registry protection rules, pipeline cancel, CI editor, LDAP/SAML group links, epic boards, and more. Only the highest-confidence cases (clear state toggle where the negative checks the disappearing old element) are included.

References

Open flaky-test tracking issues (failure::flaky-test, test-health:pass-after-retry) whose root cause this MR addresses:

The remaining specs (label subscription, registry protection rules, LDAP/SAML group links, etc.) had no open tracking issue — they are preventive fixes from a code-pattern audit of the same anti-pattern.

How to set up and validate locally

Run any of the touched specs; the reordered examples should pass reliably:

bundle exec rspec spec/features/groups/labels/subscription_spec.rb \
  spec/features/admin/admin_sees_background_migrations_spec.rb

Validated locally: subscription (group + project), admin_sees_background_migrations, usage_stats_consent, milestone, user_interacts_with_awards, and the EE LDAP/SAML group-link specs pass. RuboCop is clean on all 25 files. (Note: user_comments_on_issue_spec.rb has unrelated pre-existing BrowserConsoleError failures in its rich-text-editor shared examples — present on clean master and not touched by this MR.)

Why these regressions weren't caught at authoring time

Added after merge, as context for the GitLab Duo Code Review principle work tracked in gitlab-org/gitlab#603464. Recorded here so it cannot be argued the reviewer drew the conclusion from this description before the analysis existed.

These anti-patterns were introduced before GitLab Duo Code Review had a custom instruction that flags them, so the reviewer had no rule to catch them when the code was authored. Timeline on master:

  • .gitlab/duo/mr-review-instructions.yaml first added: 2025-06-18
  • Test Efficiency group added (which carries the positive-before-absence rule): 2026-04-20
  • The specific "confirm the page has loaded with a positive assertion before checking for absence" rule: 2026-06-11

Tracing three of the reordered cases back to the commit that introduced the negative-before-positive order:

Spec Introduced by Authored Duo absence rule available then?
spec/features/groups/labels/subscription_spec.rb fix-33991 2017-06-26 No (predates the instructions file by years)
spec/features/admin/admin_hooks_spec.rb 579392-webhooks-form-should-redirect-after-editting 2025-12-09 No (before the Test Efficiency group)
spec/features/usage_stats_consent_spec.rb remove-sign_in_form_vue-ff 2026-02-24 No (before the Test Efficiency group)

In each case the guidance that now catches the pattern did not yet exist when the code was written — these are regressions Duo could not have flagged at the time, and the rule that detects them was only added later.

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.

Edited by Pedro Pombeiro

Merge request reports