Skip to content

Simplify setup for shared runners controller spec

drew stachon requested to merge fix-flaky-runners-spec into master

What does this MR do and why?

This MR removes some suspect and definitely unnecessary let_it_be spec setup that seems like it would be responsible for the test flakiness seen in gitlab-org/quality/engineering-productivity/master-broken-incidents#2993 (closed).

The first failures (of the 20 in that job) occurred during that test, and the context only has the one test in it so the let_it_bes are kind of unnecessary environment pollution, as well as being somewhat dicey overrides of the projects that already exist. Because we can use those existing project to test this particular functionality, we should! Because the original ones are created with let_it_be in the first place!

Screenshots or screen recordings

Running this context locally reliable reproduces this failure:

% bundle exec rspec spec/controllers/projects/settings/ci_cd_controller_spec.rb:67
Run options: include {:focus=>true, :locations=>{"./spec/controllers/projects/settings/ci_cd_controller_spec.rb"=>[67]}}

Test environment set up in 6.20318 seconds
F

Failures:

  1) Projects::Settings::CiCdController as a maintainer GET show with group runners sets group runners
     Failure/Error: expect(assigns(:group_runners_count)).to be(1)
     
       expected #<Integer:3> => 1
            got #<Integer:1> => 0
     
       Compared using equal?, which compares object identity,
       but expected and actual are not the same object. Use
       `expect(actual).to eq(expected)` if you don't care about
       object identity in this example.
     # ./spec/controllers/projects/settings/ci_cd_controller_spec.rb:75:in `block (5 levels) in <top (required)>'
     # ./spec/spec_helper.rb:442:in `block (3 levels) in <top (required)>'
     # ./spec/support/sidekiq_middleware.rb:18:in `with_sidekiq_server_middleware'
     # ./spec/spec_helper.rb:433:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:429:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/application_context.rb:66:in `with_raw_context'
     # ./spec/spec_helper.rb:429:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:266:in `block (2 levels) in <top (required)>'
     # ./spec/support/system_exit_detected.rb:7:in `block (2 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:106:in `block (3 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:60:in `with_cross_joins_prevented'
     # ./spec/support/database/prevent_cross_joins.rb:106:in `block (2 levels) in <main>'

Finished in 9.87 seconds (files took 13.71 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/controllers/projects/settings/ci_cd_controller_spec.rb:72 # Projects::Settings::CiCdController as a maintainer GET show with group runners sets group runners

[TEST PROF INFO] Time spent in factories: 00:01.406 (12.27% of total time)

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

  1. bundle exec rspec spec/controllers/projects/settings/ci_cd_controller_spec.rb:67 on master
  2. bundle exec rspec spec/controllers/projects/settings/ci_cd_controller_spec.rb:67 on fix-flaky-runners-spec

MR acceptance checklist

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

Related to gitlab-org/quality/engineering-productivity/master-broken-incidents#2993 (closed)

Merge request reports