From db1925f917b30571e0e9d4de63417db90dc693d7 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Wed, 22 Nov 2017 11:12:47 +0000 Subject: Improve output for extra queries in specs Previously, this used `Array#-`, which would remove all queries that matches the query text in the original set. However, sometimes we have a problem with parameterised queries, where the query text is identical both times, so we'd run a query N times instead of once, and it would be hidden from the output. Replace the logic to only remove a given query N times from the actual log, where N is the number of times it appears in the expected log. --- spec/requests/api/merge_requests_spec.rb | 6 +++--- spec/support/query_recorder.rb | 11 +++++++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 35c6b3bb2fb..91616da6d9a 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -172,15 +172,15 @@ describe API::MergeRequests do context "when authenticated" do it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new do get api("/projects/#{project.id}/merge_requests", user) - end.count + end create(:merge_request, state: 'closed', milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) expect do get api("/projects/#{project.id}/merge_requests", user) - end.not_to exceed_query_limit(control_count) + end.not_to exceed_query_limit(control) end it "returns an array of all merge_requests" do diff --git a/spec/support/query_recorder.rb b/spec/support/query_recorder.rb index ba0b805caad..a771b50ec4c 100644 --- a/spec/support/query_recorder.rb +++ b/spec/support/query_recorder.rb @@ -69,10 +69,17 @@ RSpec::Matchers.define :exceed_query_limit do |expected| @recorder.count end + def count_queries(queries) + queries.each_with_object(Hash.new(0)) { |query, counts| counts[query] += 1 } + end + def log_message if expected.is_a?(ActiveRecord::QueryRecorder) - extra_queries = (expected.log - @recorder.log).join("\n\n") - "Extra queries: \n\n #{extra_queries}" + counts = count_queries(expected.log) + extra_queries = @recorder.log.reject { |query| counts[query] -= 1 unless counts[query].zero? } + extra_queries_display = count_queries(extra_queries).map { |query, count| "[#{count}] #{query}" } + + (['Extra queries:'] + extra_queries_display).join("\n\n") else @recorder.log_message end -- cgit v1.2.1