Skip to content

RendersCommits.prepare_commits_for_rendering test for N+1 queries is broken

In particular this test:

rspec spec/controllers/concerns/renders_commits_spec.rb:61

With:

describe '.prepare_commits_for_rendering' do
  it 'avoids N+1' do
    control = ActiveRecord::QueryRecorder.new do
      subject.prepare_commits_for_rendering(merge_request.commits.take(1))
    end

    expect do
      subject.prepare_commits_for_rendering(merge_request.commits)
      merge_request.commits.each(&:latest_pipeline)
    end.not_to exceed_all_query_limit(control.count)
  end
end

It's passing with rspec-retry but we're disabling that for controller tests in !73560 (merged) because it's not retrying it in a proper way.

Running it locally can reproduce it with:

Failures:
  1) RendersCommits.prepare_commits_for_rendering avoids N+1
     Failure/Error:
       expect do
         subject.prepare_commits_for_rendering(merge_request.commits)
         merge_request.commits.each(&:latest_pipeline)
       end.not_to exceed_all_query_limit(control.count)
       Expected a maximum of 5 queries, got 12:
     # ........................................
     # ........................................
     # ........................................
     # ./spec/controllers/concerns/renders_commits_spec.rb:67:in `block (3 levels) in <top (required)>'
     # ./spec/spec_helper.rb:416:in `block (3 levels) in <top (required)>'
     # ./spec/support/sidekiq_middleware.rb:9: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)>'
     # ./lib/gitlab/application_context.rb:31:in `with_raw_context'
     # ./spec/spec_helper.rb:403:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:207:in `block (2 levels) in <top (required)>'
     # ./spec/support/database/prevent_cross_joins.rb:102:in `block (3 levels) in <top (required)>'
     # ./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 <top (required)>'

We can also see this from CI if rspec-retry is disabled for it: https://gitlab.com/gitlab-org/gitlab/-/jobs/1750287541

I am quarantining it in !73560 (merged)