From 855235771101900a032358fe4f10713ac6b46c20 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 18 Apr 2024 15:44:44 +1000 Subject: [PATCH 1/5] Move matching logic into own class This prepares for adding more related logic there. --- lib/rspec/sql.rb | 20 ++++------------- lib/rspec/sql/query_matcher.rb | 40 ++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 16 deletions(-) create mode 100644 lib/rspec/sql/query_matcher.rb diff --git a/lib/rspec/sql.rb b/lib/rspec/sql.rb index fc76c87..f4a491e 100644 --- a/lib/rspec/sql.rb +++ b/lib/rspec/sql.rb @@ -3,7 +3,7 @@ require "active_support" require "rspec" -require_relative "sql/query_summary" +require_relative "sql/query_matcher" # We are building within the RSpec namespace for consistency and convenience. # We are not part of the RSpec team though. @@ -15,19 +15,7 @@ module Sql; end match do |block| @queries = scribe_queries(&block) - if expected.nil? - !@queries.empty? - elsif expected.is_a?(Integer) - @queries.size == expected - elsif expected.is_a?(Enumerator) && expected.inspect.match?(/:times>$/) - @queries.size == expected.size - elsif expected.is_a?(Array) - query_names == expected - elsif expected.is_a?(Hash) - query_summary == expected - else - raise "What are you expecting?" - end + matcher.matches?(expected) end failure_message do |_block| @@ -67,8 +55,8 @@ def query_descriptions @queries.map { |q| "#{q[:name]} #{q[:sql]}" } end - def query_summary - Sql::QuerySummary.new(@queries).summary + def matcher + @matcher = Sql::QueryMatcher.new(@queries) end def scribe_queries(&) diff --git a/lib/rspec/sql/query_matcher.rb b/lib/rspec/sql/query_matcher.rb new file mode 100644 index 0000000..b3971b0 --- /dev/null +++ b/lib/rspec/sql/query_matcher.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require_relative "query_summary" + +module RSpec + module Sql + # Compares expectations with actual queries. + class QueryMatcher + def initialize(queries) + @queries = queries + end + + def matches?(expected) + if expected.nil? + !@queries.empty? + elsif expected.is_a?(Integer) + @queries.size == expected + elsif expected.is_a?(Enumerator) && expected.inspect.match?(/:times>$/) + @queries.size == expected.size + elsif expected.is_a?(Array) + query_names == expected + elsif expected.is_a?(Hash) + query_summary == expected + else + raise "What are you expecting?" + end + end + + private + + def query_names + @queries.map { |q| q[:name] || q[:sql].split.take(2).join(" ") } + end + + def query_summary + QuerySummary.new(@queries).summary + end + end + end +end From 50a38c7524da20151edb50475403217713bc98e1 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 18 Apr 2024 15:59:00 +1000 Subject: [PATCH 2/5] Simplify matching logic, prepare for re-use --- lib/rspec/sql.rb | 5 ++-- lib/rspec/sql/query_matcher.rb | 48 ++++++++++++++++++++++++---------- 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/lib/rspec/sql.rb b/lib/rspec/sql.rb index f4a491e..c98e6ed 100644 --- a/lib/rspec/sql.rb +++ b/lib/rspec/sql.rb @@ -14,8 +14,9 @@ module Sql; end Matchers.define :query_database do |expected = nil| match do |block| @queries = scribe_queries(&block) + @matcher = Sql::QueryMatcher.new(@queries, expected) - matcher.matches?(expected) + matcher.matches? end failure_message do |_block| @@ -56,7 +57,7 @@ def query_descriptions end def matcher - @matcher = Sql::QueryMatcher.new(@queries) + @matcher end def scribe_queries(&) diff --git a/lib/rspec/sql/query_matcher.rb b/lib/rspec/sql/query_matcher.rb index b3971b0..67221a4 100644 --- a/lib/rspec/sql/query_matcher.rb +++ b/lib/rspec/sql/query_matcher.rb @@ -6,27 +6,47 @@ module RSpec module Sql # Compares expectations with actual queries. class QueryMatcher - def initialize(queries) + def initialize(queries, expected) @queries = queries + @expected = sanitize_number(expected) end - def matches?(expected) - if expected.nil? - !@queries.empty? - elsif expected.is_a?(Integer) - @queries.size == expected - elsif expected.is_a?(Enumerator) && expected.inspect.match?(/:times>$/) - @queries.size == expected.size - elsif expected.is_a?(Array) - query_names == expected - elsif expected.is_a?(Hash) - query_summary == expected + def matches? + # Simple presence validation. + return !@queries.empty? if expected.nil? + + actual == expected + end + + private + + attr_reader :expected + + # Support writing: `is_expected.to query_database 5.times` + def sanitize_number(expected) + if expected.is_a?(Enumerator) && expected.inspect.match?(/:times>$/) + expected.size else - raise "What are you expecting?" + expected end end - private + def actual + @actual ||= actual_compared_to_expected + end + + def actual_compared_to_expected + case expected + when Integer + @queries.size + when Array + query_names + when Hash + query_summary + else + raise "What are you expecting?" + end + end def query_names @queries.map { |q| q[:name] || q[:sql].split.take(2).join(" ") } From fc14567d692f7a45f5ff4ca95b81da93599a4f83 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 18 Apr 2024 16:59:25 +1000 Subject: [PATCH 3/5] Improve failure messages when not expecting a list --- lib/rspec/sql.rb | 13 +++++-------- lib/rspec/sql/query_matcher.rb | 12 ++++++------ spec/lib/rspec/sql_spec.rb | 28 ++++++++++++++++++++++------ 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/lib/rspec/sql.rb b/lib/rspec/sql.rb index c98e6ed..b6d505b 100644 --- a/lib/rspec/sql.rb +++ b/lib/rspec/sql.rb @@ -15,20 +15,21 @@ module Sql; end match do |block| @queries = scribe_queries(&block) @matcher = Sql::QueryMatcher.new(@queries, expected) + expected = matcher.expected matcher.matches? end failure_message do |_block| - if expected.is_a?(Enumerator) && expected.inspect.match?(/:times>$/) - expected = expected.size + if expected.nil? + return "Expected at least one database query but observed none." end <<~MESSAGE Expected database queries: #{expected} - Actual database queries: #{query_names} + Actual database queries: #{matcher.actual} - Diff: #{Expectations.differ.diff_as_object(query_names, expected)} + Diff: #{Expectations.differ.diff_as_object(matcher.actual, expected)} Full query log: @@ -48,10 +49,6 @@ def supports_block_expectations? true end - def query_names - @queries.map { |q| q[:name] || q[:sql].split.take(2).join(" ") } - end - def query_descriptions @queries.map { |q| "#{q[:name]} #{q[:sql]}" } end diff --git a/lib/rspec/sql/query_matcher.rb b/lib/rspec/sql/query_matcher.rb index 67221a4..aab3512 100644 --- a/lib/rspec/sql/query_matcher.rb +++ b/lib/rspec/sql/query_matcher.rb @@ -6,6 +6,8 @@ module RSpec module Sql # Compares expectations with actual queries. class QueryMatcher + attr_reader :expected + def initialize(queries, expected) @queries = queries @expected = sanitize_number(expected) @@ -18,9 +20,11 @@ def matches? actual == expected end - private + def actual + @actual ||= actual_compared_to_expected + end - attr_reader :expected + private # Support writing: `is_expected.to query_database 5.times` def sanitize_number(expected) @@ -31,10 +35,6 @@ def sanitize_number(expected) end end - def actual - @actual ||= actual_compared_to_expected - end - def actual_compared_to_expected case expected when Integer diff --git a/spec/lib/rspec/sql_spec.rb b/spec/lib/rspec/sql_spec.rb index c7619ff..1036f6e 100644 --- a/spec/lib/rspec/sql_spec.rb +++ b/spec/lib/rspec/sql_spec.rb @@ -67,16 +67,32 @@ ) end + it "prints user-friendly message expecting nothing" do + message = error_message { expect { User.last }.not_to query_database } + expect(message).to eq <<~TXT + Expected no database queries but observed: + + User Load SELECT "users".* FROM "users" ORDER BY "users"."id" DESC LIMIT ? + TXT + end + + it "prints user-friendly message expecting something" do + message = error_message { expect { nil }.to query_database } + expect(message).to eq( + "Expected at least one database query but observed none." + ) + end + it "prints user-friendly message expecting a number" do message = error_message { expect { User.last }.to query_database 2 } expect(message).to eq <<~TXT Expected database queries: 2 - Actual database queries: ["User Load"] + Actual database queries: 1 Diff: @@ -1 +1 @@ -2 - +["User Load"] + +1 Full query log: @@ -89,12 +105,12 @@ message = error_message { expect { User.last }.to query_database 2.times } expect(message).to eq <<~TXT Expected database queries: 2 - Actual database queries: ["User Load"] + Actual database queries: 1 Diff: @@ -1 +1 @@ -2 - +["User Load"] + +1 Full query log: @@ -134,12 +150,12 @@ # This message could be better but nobody has asked for it yet. expect(message).to eq <<~TXT Expected database queries: {:update=>{:user=>1}} - Actual database queries: ["User Load"] + Actual database queries: {:select=>{:users=>1}} Diff: @@ -1 +1 @@ -:update => {:user=>1}, - +["User Load"] + +:select => {:users=>1}, Full query log: From b3a5f26b957bd9bbb16a0747af010485c77903d7 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 18 Apr 2024 17:00:32 +1000 Subject: [PATCH 4/5] Stop rspec from printing unnecessary example performance --- spec/spec_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 15329d3..890a86f 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -80,7 +80,7 @@ # Print the 10 slowest examples and example groups at the # end of the spec run, to help surface which specs are running # particularly slow. - config.profile_examples = 10 + # config.profile_examples = 10 # Run specs in random order to surface order dependencies. If you find an # order dependency and want to debug it, you can fix the order by providing From e5325d637bee760a4afe8071b642695b06dcad01 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 19 Apr 2024 09:21:51 +1000 Subject: [PATCH 5/5] Show relative difference of numbers of queries --- lib/rspec/sql.rb | 11 ++++++++++- spec/lib/rspec/sql_spec.rb | 16 ++++------------ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/rspec/sql.rb b/lib/rspec/sql.rb index b6d505b..6aa3d75 100644 --- a/lib/rspec/sql.rb +++ b/lib/rspec/sql.rb @@ -29,7 +29,7 @@ module Sql; end Expected database queries: #{expected} Actual database queries: #{matcher.actual} - Diff: #{Expectations.differ.diff_as_object(matcher.actual, expected)} + Diff: #{diff(matcher.actual, expected)} Full query log: @@ -57,6 +57,15 @@ def matcher @matcher end + def diff(actual, expected) + if expected.is_a?(Numeric) + change = actual - expected + format("%+d", change) + else + Expectations.differ.diff_as_object(actual, expected) + end + end + def scribe_queries(&) queries = [] diff --git a/spec/lib/rspec/sql_spec.rb b/spec/lib/rspec/sql_spec.rb index 1036f6e..5ea1ffc 100644 --- a/spec/lib/rspec/sql_spec.rb +++ b/spec/lib/rspec/sql_spec.rb @@ -84,16 +84,12 @@ end it "prints user-friendly message expecting a number" do - message = error_message { expect { User.last }.to query_database 2 } + message = error_message { expect { User.last }.to query_database 0 } expect(message).to eq <<~TXT - Expected database queries: 2 + Expected database queries: 0 Actual database queries: 1 - Diff: - @@ -1 +1 @@ - -2 - +1 - + Diff: +1 Full query log: @@ -107,11 +103,7 @@ Expected database queries: 2 Actual database queries: 1 - Diff: - @@ -1 +1 @@ - -2 - +1 - + Diff: -1 Full query log: