Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error message for n.times syntax #4

Merged
merged 4 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ jobs:
ruby-version: ${{ matrix.ruby-version }}
bundler-cache: true
- run: bundle exec rspec
- run: bundle exec rubocop
31 changes: 31 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
AllCops:
NewCops: enable

Layout/LineLength:
Max: 80 # default is 120

Metrics/BlockLength:
AllowedMethods:
# RSpec context block containing many smaller blocks:
- Matchers.define
# RSpec context block containing many smaller examples:
- RSpec.describe

Style/BlockDelimiters:
BracesRequiredMethods:
# The RSpec expect syntax is prettier with braced blocks.
- expect

Style/StringLiterals:
# It's my preference to add or remove variable from strings without replacing
# the outer quotes.
EnforcedStyle: double_quotes

Style/TrailingCommaInArrayLiteral:
# Consistent commas allow for adding and removing lines without affecting
# other lines of arrays. Less editing, prettier diffs.
EnforcedStyleForMultiline: consistent_comma

Style/TrailingCommaInHashLiteral:
# Same reason as above.
EnforcedStyleForMultiline: consistent_comma
3 changes: 3 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# frozen_string_literal: true

source "https://rubygems.org"
gemspec

gem "activerecord"
gem "rubocop", require: false
gem "sqlite3"
27 changes: 27 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ GEM
minitest (>= 5.1)
mutex_m
tzinfo (~> 2.0)
ast (2.4.2)
base64 (0.2.0)
bigdecimal (3.1.6)
concurrent-ruby (1.2.3)
Expand All @@ -33,8 +34,18 @@ GEM
ruby2_keywords
i18n (1.14.1)
concurrent-ruby (~> 1.0)
json (2.7.2)
language_server-protocol (3.17.0.3)
minitest (5.22.2)
mutex_m (0.2.0)
parallel (1.24.0)
parser (3.3.0.5)
ast (~> 2.4.1)
racc
racc (1.7.3)
rainbow (3.1.1)
regexp_parser (2.9.0)
rexml (3.2.6)
rspec (3.13.0)
rspec-core (~> 3.13.0)
rspec-expectations (~> 3.13.0)
Expand All @@ -48,18 +59,34 @@ GEM
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.13.0)
rspec-support (3.13.0)
rubocop (1.63.1)
json (~> 2.3)
language_server-protocol (>= 3.17.0)
parallel (~> 1.10)
parser (>= 3.3.0.2)
rainbow (>= 2.2.2, < 4.0)
regexp_parser (>= 1.8, < 3.0)
rexml (>= 3.2.5, < 4.0)
rubocop-ast (>= 1.31.1, < 2.0)
ruby-progressbar (~> 1.7)
unicode-display_width (>= 2.4.0, < 3.0)
rubocop-ast (1.31.2)
parser (>= 3.3.0.4)
ruby-progressbar (1.13.0)
ruby2_keywords (0.0.5)
sqlite3 (1.7.2-x86_64-linux)
timeout (0.4.1)
tzinfo (2.0.6)
concurrent-ruby (~> 1.0)
unicode-display_width (2.5.0)

PLATFORMS
x86_64-linux

DEPENDENCIES
activerecord
rspec-sql!
rubocop
sqlite3

BUNDLED WITH
Expand Down
19 changes: 15 additions & 4 deletions lib/rspec/sql.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@

require_relative "sql/query_summary"

# We are building within the RSpec namespace for consistency and convenience.
# We are not part of the RSpec team though.
module RSpec
# RSpec::Sql contains our code.
module Sql; end

RSpec::Matchers.define :query_database do |expected = nil|
Matchers.define :query_database do |expected = nil|
match do |block|
@queries = scribe_queries(&block)

Expand All @@ -28,6 +31,10 @@ module Sql; end
end

failure_message do |_block|
if expected.is_a?(Enumerator) && expected.inspect.match?(/:times>$/)
expected = expected.size
end

Comment on lines +34 to +37
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking today that we should create a new object:

@matcher = QueryMatcher.new(@queries)
@matcher.matches?(expected)

That matcher would implement the different cases of comparing to different types. It would be easier to test that class and by storing it in an ivar, we can re-use it to print the error message.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds like a good idea 👍

<<~MESSAGE
Expected database queries: #{expected}
Actual database queries: #{query_names}
Expand All @@ -41,7 +48,11 @@ module Sql; end
end

failure_message_when_negated do |_block|
"Expected no database queries but observed:\n\n#{query_descriptions.join("\n")}"
<<~TXT
Expected no database queries but observed:

#{query_descriptions.join("\n")}
TXT
end

def supports_block_expectations?
Expand All @@ -63,9 +74,9 @@ def query_summary
def scribe_queries(&)
queries = []

logger = ->(_name, _started, _finished, _unique_id, payload) {
logger = lambda do |_name, _started, _finished, _unique_id, payload|
queries << payload unless %w[CACHE SCHEMA].include?(payload[:name])
}
end

ActiveSupport::Notifications.subscribed(logger, "sql.active_record", &)

Expand Down
3 changes: 2 additions & 1 deletion lib/rspec/sql/query_summary.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

module RSpec
module Sql
# Converts a list of queries into a summary hash.
class QuerySummary
QUERY_TYPES = [:delete, :insert, :select, :update].freeze
QUERY_TYPES = %i[delete insert select update].freeze

attr_reader :summary

Expand Down
5 changes: 4 additions & 1 deletion rspec-sql.gemspec
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

Gem::Specification.new do |s|
s.name = "rspec-sql"
s.version = "0.0.1"
Expand All @@ -8,8 +10,9 @@ Gem::Specification.new do |s|
s.files = Dir["lib/**/*.rb"]
s.homepage = "https://github.com/openfoodfoundation/rspec-sql"
s.license = "AGPL-3.0-or-later"
s.required_ruby_version = '>= 3.1', '< 4'
s.required_ruby_version = ">= 3.1", "< 4"

s.add_runtime_dependency "activesupport"
s.add_runtime_dependency "rspec"
s.metadata["rubygems_mfa_required"] = "true"
end
88 changes: 88 additions & 0 deletions spec/lib/rspec/sql_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,92 @@
}
)
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"]

Diff:
@@ -1 +1 @@
-2
+["User Load"]


Full query log:

User Load SELECT "users".* FROM "users" ORDER BY "users"."id" DESC LIMIT ?
TXT
end

it "prints user-friendly message expecting x.times" do
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"]

Diff:
@@ -1 +1 @@
-2
+["User Load"]


Full query log:

User Load SELECT "users".* FROM "users" ORDER BY "users"."id" DESC LIMIT ?
TXT
end

it "prints user-friendly message expecting list" do
message = error_message do
expect { User.last }.to query_database ["User Update"]
end

expect(message).to eq <<~TXT
Expected database queries: ["User Update"]
Actual database queries: ["User Load"]

Diff:
@@ -1 +1 @@
-["User Update"]
+["User Load"]


Full query log:

User Load SELECT "users".* FROM "users" ORDER BY "users"."id" DESC LIMIT ?
TXT
end

it "prints user-friendly message expecting summary" do
message = error_message do
expect { User.last }.to query_database(
update: { user: 1 }
)
end

# 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"]

Diff:
@@ -1 +1 @@
-:update => {:user=>1},
+["User Load"]


Full query log:

User Load SELECT "users".* FROM "users" ORDER BY "users"."id" DESC LIMIT ?
TXT
end

def error_message
yield
rescue RSpec::Expectations::ExpectationNotMetError => e
# Remove colours and trailing whitespace from message:
e.message.gsub(/\e\[(\d+)m/, "").gsub(/ $/, "")
end
end
Loading