From 22c4f879cbea0dc4fc48fd88f317b4345a27f953 Mon Sep 17 00:00:00 2001 From: Vladimir Dementyev Date: Fri, 17 Feb 2023 11:56:42 -0500 Subject: [PATCH] refactor: exact matching --- CHANGELOG.md | 17 +++++ README.md | 11 +++ lib/n_plus_one_control/minitest.rb | 31 ++------ .../perform_constant_number_of_queries.rb | 8 ++- .../perform_linear_number_of_queries.rb | 2 - tests/minitest_test.rb | 70 +++++++++---------- 6 files changed, 73 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cf3c31..c4f3cef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ ## master +- Added ability to specify the exact number of expected queries when using constant matchers. ([@akostadinov][], [@palkan][]) + +For RSpec, you can add the `.exactly` modifier: + +```ruby +expect { get :index }.to perform_constant_number_of_queries.exactly(1) +``` + +For Minitest, you can provide the expected number of queries as the first argument: + +```ruby +assert_perform_constant_number_of_queries(0, **options) do + get :index +end +``` + - **Require Ruby 2.7+**. ## 0.6.2 (2021-10-26) @@ -46,3 +62,4 @@ Could be specified via `NPLUSONE_BACKTRACE` env var. [@palkan]: https://github.com/palkan [@caalberts]: https://github.com/caalberts [@andrewhampton]: https://github.com/andrewhampton +[@akostadinov]: https://github.com/akostadinov diff --git a/README.md b/README.md index e467f49..0b47c01 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,9 @@ expect { get :index }.to perform_constant_number_of_queries.matching(/INSERT/) # You can also provide custom scale factors expect { get :index }.to perform_constant_number_of_queries.with_scale_factors(10, 100) + +# You can specify the exact number of expected queries +expect { get :index }.to perform_constant_number_of_queries.exactly(1) ``` #### Using scale factor in spec @@ -214,6 +217,14 @@ assert_perform_constant_number_of_queries( end ``` +For the constant matcher, you can also specify the expected number of queries as the first argument: + +```ruby +assert_perform_constant_number_of_queries(2, populate: populate) do + get :index +end +``` + It's possible to specify a filter via `NPLUSONE_FILTER` env var, e.g.: ```ruby diff --git a/lib/n_plus_one_control/minitest.rb b/lib/n_plus_one_control/minitest.rb index 017dd68..3b1c903 100644 --- a/lib/n_plus_one_control/minitest.rb +++ b/lib/n_plus_one_control/minitest.rb @@ -5,31 +5,8 @@ module NPlusOneControl # Minitest assertions module MinitestHelper - def assert_number_of_queries( - number, - populate: nil, - matching: nil, - warmup: nil - ) - - raise ArgumentError, "Block is required" unless block_given? - - warming_up warmup - - @executor = NPlusOneControl::Executor.new( - population: populate || population_method, - matching: (matching || NPlusOneControl.default_matching), - scale_factors: [1] - ) - - queries = @executor.call { yield } - - counts = queries.map(&:last).map(&:size) - - assert_equal number, counts.max, NPlusOneControl.failure_message(:number_of_queries, queries) - end - def assert_perform_constant_number_of_queries( + exact = nil, populate: nil, matching: nil, scale_factors: nil, @@ -50,7 +27,11 @@ def assert_perform_constant_number_of_queries( counts = queries.map(&:last).map(&:size) - assert counts.max == counts.min, NPlusOneControl.failure_message(:constant_queries, queries) + if exact + assert counts.all? { _1 == exact }, NPlusOneControl.failure_message(:number_of_queries, queries) + else + assert counts.max == counts.min, NPlusOneControl.failure_message(:constant_queries, queries) + end end def assert_perform_linear_number_of_queries( diff --git a/lib/n_plus_one_control/rspec/matchers/perform_constant_number_of_queries.rb b/lib/n_plus_one_control/rspec/matchers/perform_constant_number_of_queries.rb index f3ed18b..e5154e8 100644 --- a/lib/n_plus_one_control/rspec/matchers/perform_constant_number_of_queries.rb +++ b/lib/n_plus_one_control/rspec/matchers/perform_constant_number_of_queries.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# rubocop:disable Metrics/BlockLength ::RSpec::Matchers.define :perform_constant_number_of_queries do supports_block_expectations @@ -43,7 +42,11 @@ counts = @queries.map(&:last).map(&:size) - counts.max == (@exactly || counts.min) + if @exactly + counts.all? { _1 == @exactly } + else + counts.max == counts.min + end end match_when_negated do |_actual| @@ -52,4 +55,3 @@ failure_message { |_actual| NPlusOneControl.failure_message(@exactly ? :number_of_queries : :constant_queries, @queries) } end -# rubocop:enable Metrics/BlockLength diff --git a/lib/n_plus_one_control/rspec/matchers/perform_linear_number_of_queries.rb b/lib/n_plus_one_control/rspec/matchers/perform_linear_number_of_queries.rb index 38351df..d19f3a0 100644 --- a/lib/n_plus_one_control/rspec/matchers/perform_linear_number_of_queries.rb +++ b/lib/n_plus_one_control/rspec/matchers/perform_linear_number_of_queries.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# rubocop:disable Metrics/BlockLength ::RSpec::Matchers.define :perform_linear_number_of_queries do |slope: 1| supports_block_expectations @@ -50,4 +49,3 @@ failure_message { |_actual| NPlusOneControl.failure_message(:linear_queries, @queries) } end -# rubocop:enable Metrics/BlockLength diff --git a/tests/minitest_test.rb b/tests/minitest_test.rb index f89583e..d36aa5e 100644 --- a/tests/minitest_test.rb +++ b/tests/minitest_test.rb @@ -2,42 +2,6 @@ require_relative "test_helper" -class TestMinitestSpecifiedQueries < Minitest::Test - def test_queries_match - populate = ->(n) { create_list(:post, n) } - - assert_number_of_queries(1, populate: populate) do - Post.take - end - end - - def test_queries_do_not_match - populate = ->(n) { create_list(:post, n) } - - e = assert_raises Minitest::Assertion do - assert_number_of_queries(0, populate: populate) do - Post.take - end - end - - assert_match "Expected to make the specified number of queries", e.message - assert_match "1 for N=1", e.message - assert_match(/posts \(SELECT\): 1$/, e.message) - end - - def test_no_n_plus_one_error_with_matching - populate = ->(n) { create_list(:post, n) } - - assert_number_of_queries( - 1, - populate: populate, - matching: /posts/ - ) do - Post.find_each { |p| p.user.name } - end - end -end - class TestMinitestConstantQueries < Minitest::Test def test_no_n_plus_one_error populate = ->(n) { create_list(:post, n) } @@ -96,6 +60,40 @@ def test_no_n_plus_one_error_with_warmup assert warmed_up end + + def test_exact_number_queries_match + populate = ->(n) { create_list(:post, n) } + + assert_perform_constant_number_of_queries(1, populate: populate) do + Post.take + end + end + + def test_exact_number_queries_do_not_match + populate = ->(n) { create_list(:post, n) } + + e = assert_raises Minitest::Assertion do + assert_perform_constant_number_of_queries(0, populate: populate, scale_factors: [1]) do + Post.take + end + end + + assert_match "Expected to make the specified number of queries", e.message + assert_match "1 for N=1", e.message + assert_match(/posts \(SELECT\): 1$/, e.message) + end + + def test_exact_number_error_with_matching + populate = ->(n) { create_list(:post, n) } + + assert_perform_constant_number_of_queries( + 1, + populate: populate, + matching: /posts/ + ) do + Post.find_each { |p| p.user.name } + end + end end class TestMinitestLinearQueries < Minitest::Test