Skip to content

Fix flaky spec by using explicit value

Steve Abrams requested to merge 344951-flaky-dp-spec into master

🔍 What does this MR do and why?

The dependency_proxy_for_containers_controller_spec contains a hardcoded value to match a factory value, but the factory value is a dynamic sequence. If this test is run after a different test containing one of those objects is created, then it will fail. This caused the master:broken issue #344951 (closed).

This MR updates the spec to use an explicit value for the failing set of tests instead of the dynamic sequence, fixing the problem.

📷 Screenshots or screen recordings

Before:

→ bin/rspec spec/requests/rack_attack_global_spec.rb:514 spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb:242
Run options: include {:focus=>true, :locations=>{"./spec/requests/rack_attack_global_spec.rb"=>[514], "./spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb"=>[242]}}
WARNING: Gitaly, version 14.4.0-rc43-84-g80d41b905 does not exactly match repository version 59ec40d9ed6d719bcccc77193e031ffee3098afc
==> Gitaly set up in 61.150011 seconds...

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

Failures:

  1) Groups::DependencyProxyForContainersController GET #manifest feature enabled a valid user with workhorse response returns Workhorse send-dependency instructions
     Failure/Error:
       expect(response.headers['Content-Disposition']).to eq(
         ActionDispatch::Http::ContentDisposition.format(disposition: 'attachment', filename: manifest.file_name)
       )

       expected: "attachment; filename=\"alpine%3Alatest2.json\"; filename*=UTF-8''alpine%3Alatest2.json"
            got: "attachment; filename=\"alpine%3Alatest1.json\"; filename*=UTF-8''alpine%3Alatest1.json"

       (compared using ==)
     # ./spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb:255:in `block (6 levels) in <main>'
     # ./spec/spec_helper.rb:408:in `block (3 levels) in <top (required)>'
     # ./spec/support/sidekiq_middleware.rb:9:in `with_sidekiq_server_middleware'
     # ./spec/spec_helper.rb:399:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:395:in `block (3 levels) in <top (required)>'
     # ./lib/gitlab/application_context.rb:31:in `with_raw_context'
     # ./spec/spec_helper.rb:395:in `block (2 levels) in <top (required)>'
     # ./spec/support/database/prevent_cross_joins.rb:102:in `block (3 levels) in <main>'
     # ./spec/support/database/prevent_cross_joins.rb:56:in `with_cross_joins_prevented'
     # ./spec/support/database/prevent_cross_joins.rb:102:in `block (2 levels) in <main>'

Finished in 1 minute 39.1 seconds (files took 26.34 seconds to load)
11 examples, 1 failure

Failed examples:

rspec ./spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb:242 # Groups::DependencyProxyForContainersController GET #manifest feature enabled a valid user with workhorse response returns Workhorse send-dependency instructions

After

→ bin/rspec spec/requests/rack_attack_global_spec.rb:514 spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb:242
Run options: include {:focus=>true, :locations=>{"./spec/requests/rack_attack_global_spec.rb"=>[514], "./spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb"=>[242]}}
WARNING: Gitaly, version 14.4.0-rc43-84-g80d41b905 does not exactly match repository version 59ec40d9ed6d719bcccc77193e031ffee3098afc
==> Gitaly set up in 60.197751 seconds...

Test environment set up in 73.807491 seconds
...........

Finished in 1 minute 37.83 seconds (files took 25.93 seconds to load)
11 examples, 0 failures

💻 How to set up and validate locally

Run these two tests in order:

# short version
bin/rspec spec/requests/rack_attack_global_spec.rb:514 spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb:242


# long version
bin/rspec spec/requests/rack_attack_global_spec.rbspec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb

All tests should pass with this update.

📏 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 #344951 (closed)

Edited by Steve Abrams

Merge request reports

Loading