Skip to content

Avoid retrying controller tests due to data not being reset

Lin Jen-Shin requested to merge dont-retry-controller-tests into master

What does this MR do and why?

Do not retry controller tests because rspec-retry cannot properly reset the controller which may contain data from last attempt.

See !73360 (merged)

  • Don't squash this merge request because each commits are fixing different tests

Hidden broken tests

  • https://gitlab.com/gitlab-org/gitlab/-/jobs/1740470213
    • This is concerning that it's expecting a worker is not performing, yet it's performing. Upon retrying then it's not performing and it's passing.
    • This is because the maintainer can push to the fork due to having allow_maintainer_to_push as true by default

How to set up and validate locally

  1. Revert !73360 (merged) locally so we have a broken controller test to try.
  2. Run env RETRIES=1 rspec 'spec/controllers/projects/merge_requests_controller_spec.rb[1:8:2:3:1:1:1]' and see it fails. (expected, because it's broken)
  3. Run env RETRIES=2 rspec 'spec/controllers/projects/merge_requests_controller_spec.rb[1:8:2:3:1:1:1]' and see it passes. (not expected, because retrying something broken should not pass)
  4. Apply this merge request.
  5. Run env RETRIES=2 rspec 'spec/controllers/projects/merge_requests_controller_spec.rb[1:8:2:3:1:1:1]' and see it fails. (expected, because now we never retry controller tests)

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Lin Jen-Shin

Merge request reports