Simplify setup for shared runners controller spec
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_be
s 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.
-
bundle exec rspec spec/controllers/projects/settings/ci_cd_controller_spec.rb:67
onmaster
-
bundle exec rspec spec/controllers/projects/settings/ci_cd_controller_spec.rb:67
onfix-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.
-
I have evaluated the MR acceptance checklist for this MR.
Related to gitlab-org/quality/engineering-productivity/master-broken-incidents#2993 (closed)