Skip to content

Improve output for extra queries in specs

Sean McGivern requested to merge improve-extra-queries-output into master

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14267 made a cool change, but Array#- made the output less useful than it could be. Say I make this change:

diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 726f09e3669..a4eb51db0bc 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -21,7 +21,7 @@ module API
         return merge_requests if args[:view] == 'simple'

         merge_requests
-          .preload(:notes, :author, :assignee, :milestone, :merge_request_diff, :labels, :timelogs)
+          .preload(:notes, :author, :assignee, :milestone, :labels, :timelogs)
       end

       params :merge_requests_params do

And run the MR API spec I changed in this MR:

API::MergeRequests
  GET /projects/:id/merge_requests
    when authenticated
      avoids N+1 queries (FAILED - 1)

Failures:

  1) API::MergeRequests GET /projects/:id/merge_requests when authenticated avoids N+1 queries
     Failure/Error:
       expect do
         get api("/projects/#{project.id}/merge_requests", user)
       end.not_to exceed_query_limit(control)

       Expected a maximum of 25 queries, got 26:

       Extra queries:

        SELECT "notes".* FROM "notes" WHERE "notes"."noteable_type" = 'MergeRequest' AND "notes"."noteable_id" IN (3, 2, 1)

       SELECT "label_links".* FROM "label_links" WHERE "label_links"."target_type" = 'MergeRequest' AND "label_links"."target_id" IN (3, 2, 1)

       SELECT "timelogs".* FROM "timelogs" WHERE "timelogs"."merge_request_id" IN (3, 2, 1)

       SELECT "award_emoji"."name", "award_emoji"."awardable_id", COUNT(*) as count FROM "award_emoji" WHERE (name IN ('thumbsdown','thumbsup') AND awardable_type = 'MergeRequest' AND awardable_id IN (3,2,1)) GROUP BY "award_emoji"."name", "award_emoji"."awardable_id"

       SELECT "notes"."noteable_id", COUNT(*) as count FROM "notes" WHERE "notes"."system" = $1 AND "notes"."noteable_type" = $2 AND "notes"."noteable_id" IN (3, 2, 1) GROUP BY "notes"."noteable_id"
     # ./spec/requests/api/merge_requests_spec.rb:181:in `block (4 levels) in <top (required)>'
     # -e:1:in `<main>'

Finished in 13.29 seconds (files took 1.17 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/requests/api/merge_requests_spec.rb:174 # API::MergeRequests GET /projects/:id/merge_requests when authenticated avoids N+1 queries

That's not very useful - none of those queries were added, they were just changed because we're preloading for one more MR now.

After this change, running the same spec:

API::MergeRequests
  GET /projects/:id/merge_requests
    when authenticated
      avoids N+1 queries (FAILED - 1)

Failures:

  1) API::MergeRequests GET /projects/:id/merge_requests when authenticated avoids N+1 queries
     Failure/Error:
       expect do
         get api("/projects/#{project.id}/merge_requests", user)
       end.not_to exceed_query_limit(control)

       Expected a maximum of 25 queries, got 26:

       Extra queries:

       SELECT "notes".* FROM "notes" WHERE "notes"."noteable_type" = 'MergeRequest' AND "notes"."noteable_id" IN (3, 2, 1, 4)

       SELECT "label_links".* FROM "label_links" WHERE "label_links"."target_type" = 'MergeRequest' AND "label_links"."target_id" IN (3, 2, 1, 4)

       SELECT "timelogs".* FROM "timelogs" WHERE "timelogs"."merge_request_id" IN (3, 2, 1, 4)

       SELECT "award_emoji"."name", "award_emoji"."awardable_id", COUNT(*) as count FROM "award_emoji" WHERE (name IN ('thumbsdown','thumbsup') AND awardable_type = 'MergeRequest' AND awardable_id IN (3,2,1,4)) GROUP BY "award_emoji"."name", "award_emoji"."awardable_id"

       SELECT "notes"."noteable_id", COUNT(*) as count FROM "notes" WHERE "notes"."system" = $1 AND "notes"."noteable_type" = $2 AND "notes"."noteable_id" IN (3, 2, 1, 4) GROUP BY "notes"."noteable_id"

       SELECT  "merge_request_diffs".* FROM "merge_request_diffs" WHERE "merge_request_diffs"."merge_request_id" = $1  ORDER BY merge_request_diffs.id DESC LIMIT 1
     # ./spec/requests/api/merge_requests_spec.rb:181:in `block (4 levels) in <top (required)>'
     # -e:1:in `<main>'

Finished in 13.06 seconds (files took 1.09 seconds to load)
1 example, 1 failure

Failed examples:

rspec ./spec/requests/api/merge_requests_spec.rb:174 # API::MergeRequests GET /projects/:id/merge_requests when authenticated avoids N+1 queries

The first ones aren't very useful, but the last one is - looks like we added an extra query to get the MR diffs somewhere! Better preload it!

Merge request reports