Skip to content

Remove flake-inducing usage of let_it_be

drew stachon requested to merge fix-flaky-chain-validate-spec into master

What does this MR do and why?

This MR removes a usage of let_it_be that's causing unexpected test failures that depend on how many tests are run together.

I discovered this by adding another spec in !107033 (closed) that I expected to fail, which succeeded but produced this unexpected failure on line 161 when the spec file was isolated by rspec fail-fast

I also added a feature category since I kept getting warnings that this file was missing one. I think it's grouppipeline execution but I'll get someone from grouppipeline authoring to review this, just in case they disagree with me 😄

Screenshots or screen recordings

Currently

Failing in a run of the full spec file:

$ bundle exec rspec spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}

Test environment set up in 8.500544 seconds
.............F.....

Failures:

  1) Gitlab::Ci::Pipeline::Chain::Validate::Abilities#allowed_to_write_ref? when user is a maintainer is expected to be truthy
     Failure/Error: it { is_expected.to be_truthy }
     
       expected: truthy value
            got: false
     # ./spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb:161:in `block (4 levels) in <top (required)>'
     # ./spec/spec_helper.rb:415:in `block (3 levels) in <top (required)>'
     # /Users/drew/.rvm/gems/ruby-3.0.5/gems/sidekiq-6.5.7/lib/sidekiq/testing.rb:55:in `server_middleware'
     # ./spec/support/sidekiq_middleware.rb:18:in `with_sidekiq_server_middleware'
     # ./spec/spec_helper.rb:407:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:403:in `block (3 levels) in <top (required)>'
     # /Users/drew/.rvm/gems/ruby-3.0.5/gems/gitlab-labkit-0.29.0/lib/labkit/context.rb:36:in `with_context'
     # ./lib/gitlab/application_context.rb:59:in `with_raw_context'
     # ./spec/spec_helper.rb:403:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:239:in `block (2 levels) in <top (required)>'
     # /Users/drew/.rvm/gems/ruby-3.0.5/gems/webmock-3.9.1/lib/webmock/rspec.rb:37:in `block (2 levels) in <main>'
     # ./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>'
     # /Users/drew/.rvm/gems/ruby-3.0.5/gems/rspec-retry-0.6.1/lib/rspec/retry.rb:123:in `block in run'
     # /Users/drew/.rvm/gems/ruby-3.0.5/gems/rspec-retry-0.6.1/lib/rspec/retry.rb:110:in `loop'
     # /Users/drew/.rvm/gems/ruby-3.0.5/gems/rspec-retry-0.6.1/lib/rspec/retry.rb:110:in `run'
     # /Users/drew/.rvm/gems/ruby-3.0.5/gems/rspec-retry-0.6.1/lib/rspec_ext/rspec_ext.rb:12:in `run_with_retry'
     # /Users/drew/.rvm/gems/ruby-3.0.5/gems/rspec-retry-0.6.1/lib/rspec/retry.rb:37:in `block (2 levels) in setup'

Finished in 21.94 seconds (files took 20.06 seconds to load)
19 examples, 1 failure

Failed examples:

rspec ./spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb:161 # Gitlab::Ci::Pipeline::Chain::Validate::Abilities#allowed_to_write_ref? when user is a maintainer is expected to be truthy

But succeeding when run individually:

$ bundle exec rspec spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb:161
Run options: include {:focus=>true, :locations=>{"./spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb"=>[161]}}

Test environment set up in 9.82401 seconds
.

Finished in 16.67 seconds (files took 18.36 seconds to load)
1 example, 0 failures

After this change

The full spec file passes:

$ bundle exec rspec spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}

Test environment set up in 8.559183 seconds
...................

Finished in 1 minute 13.12 seconds (files took 18.06 seconds to load)
19 examples, 0 failures

and the individual spec still passes:

$ bundle exec rspec spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb:161
Run options: include {:focus=>true, :locations=>{"./spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb"=>[161]}}

Test environment set up in 10.395173 seconds
.

Finished in 17.4 seconds (files took 20.88 seconds to load)
1 example, 0 failures

How to set up and validate locally

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

MR acceptance checklist

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

Merge request reports