From 8392c54e73087991eb94ca7e3d2ee4789bed96e0 Mon Sep 17 00:00:00 2001 From: Petrik Date: Wed, 1 Mar 2023 13:57:02 +0100 Subject: [PATCH] Expose `assert_queries` and `assert_no_queries` assertions To assert the expected number of queries are made, Rails internally uses `assert_queries` and `assert_no_queries`. These assertions can be useful in applications as well. By extracting these assertions to a module, the assertions can be included where required. These assertions are added to `ActiveSupport::TestCase` when ActiveRecord is defined. ActiveStorage, ActionView and ActionText are using this module now as well, instead of duplicating the implementation. The internal ActiveRecord::TestCase, used for testing ActiveRecord, implements these assertions as well. However, these are slighlty more advanced/complex and use the SQLCounter class. To keep things simple, for now this implementation isn't used. --- actiontext/test/test_helper.rb | 19 ++-------- .../test/activerecord/relation_cache_test.rb | 15 ++------ activerecord/CHANGELOG.md | 16 +++++++++ .../active_record/testing/query_assertions.rb | 24 +++++++++++++ .../cases/assertions/query_assertions_test.rb | 36 +++++++++++++++++++ activestorage/test/test_helper.rb | 20 ++--------- railties/lib/rails/test_help.rb | 3 +- 7 files changed, 85 insertions(+), 48 deletions(-) create mode 100644 activerecord/lib/active_record/testing/query_assertions.rb create mode 100644 activerecord/test/cases/assertions/query_assertions_test.rb diff --git a/actiontext/test/test_helper.rb b/actiontext/test/test_helper.rb index 372fc070732..86735bcc518 100644 --- a/actiontext/test/test_helper.rb +++ b/actiontext/test/test_helper.rb @@ -6,6 +6,7 @@ require "active_support/testing/strict_warnings" ENV["RAILS_ENV"] = "test" require_relative "../test/dummy/config/environment" +require "active_record/testing/query_assertions" ActiveRecord::Migrator.migrations_paths = [File.expand_path("../test/dummy/db/migrate", __dir__)] require "rails/test_help" @@ -26,23 +27,7 @@ end class ActiveSupport::TestCase module QueryHelpers include ActiveJob::TestHelper - - def assert_queries(expected_count, &block) - ActiveRecord::Base.connection.materialize_transactions - - queries = [] - ActiveSupport::Notifications.subscribe("sql.active_record") do |*, payload| - queries << payload[:sql] unless %w[ SCHEMA TRANSACTION ].include?(payload[:name]) - end - - result = _assert_nothing_raised_or_warn("assert_queries", &block) - assert_equal expected_count, queries.size, "#{queries.size} instead of #{expected_count} queries were executed. #{queries.inspect}" - result - end - - def assert_no_queries(&block) - assert_queries(0, &block) - end + include ActiveRecord::Assertions::QueryAssertions end private diff --git a/actionview/test/activerecord/relation_cache_test.rb b/actionview/test/activerecord/relation_cache_test.rb index 7d029affde1..98fcccf80e9 100644 --- a/actionview/test/activerecord/relation_cache_test.rb +++ b/actionview/test/activerecord/relation_cache_test.rb @@ -1,9 +1,11 @@ # frozen_string_literal: true require "active_record_unit" +require "active_record/testing/query_assertions" class RelationCacheTest < ActionView::TestCase tests ActionView::Helpers::CacheHelper + include ActiveRecord::Assertions::QueryAssertions def setup super @@ -24,17 +26,4 @@ class RelationCacheTest < ActionView::TestCase end def view_cache_dependencies; []; end - - def assert_queries(num, &block) - ActiveRecord::Base.connection.materialize_transactions - count = 0 - - ActiveSupport::Notifications.subscribe("sql.active_record") do |_name, _start, _finish, _id, payload| - count += 1 unless ["SCHEMA", "TRANSACTION"].include? payload[:name] - end - - result = _assert_nothing_raised_or_warn("assert_queries", &block) - assert_equal num, count, "#{count} instead of #{num} queries were executed." - result - end end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 49fe1433274..44ff50abd8a 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,19 @@ +* Make `assert_queries` and `assert_no_queries` assertions public. + + To assert the expected number of queries are made, Rails internally uses + `assert_queries` and `assert_no_queries`. These assertions can be now + be used in applications as well. + + ```ruby + class ArticleTest < ActiveSupport::TestCase + test "queries are made" do + assert_queries(1) { Article.first } + end + end + ``` + + *Petrik de Heus* + * Fix `has_secure_token` calls the setter method on initialize. *Abeid Ahmed* diff --git a/activerecord/lib/active_record/testing/query_assertions.rb b/activerecord/lib/active_record/testing/query_assertions.rb new file mode 100644 index 00000000000..ab558d78194 --- /dev/null +++ b/activerecord/lib/active_record/testing/query_assertions.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module ActiveRecord + module Assertions + module QueryAssertions + def assert_queries(expected_count, matcher: nil, &block) + ActiveRecord::Base.connection.materialize_transactions + + queries = [] + ActiveSupport::Notifications.subscribe("sql.active_record") do |*, payload| + queries << payload[:sql] if %w[ SCHEMA TRANSACTION ].exclude?(payload[:name]) && (matcher.nil? || payload[:sql].match(matcher)) + end + + result = _assert_nothing_raised_or_warn("assert_queries", &block) + assert_equal expected_count, queries.size, "#{queries.size} instead of #{expected_count} queries were executed. Queries: #{queries.join("\n\n")}" + result + end + + def assert_no_queries(&block) + assert_queries(0, &block) + end + end + end +end diff --git a/activerecord/test/cases/assertions/query_assertions_test.rb b/activerecord/test/cases/assertions/query_assertions_test.rb new file mode 100644 index 00000000000..8dcbadb4abe --- /dev/null +++ b/activerecord/test/cases/assertions/query_assertions_test.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require "cases/helper" +require "models/post" +require "active_record/testing/query_assertions" + +module ActiveRecord + module Assertions + class QueryAssertionsTest < ActiveSupport::TestCase + include QueryAssertions + + def test_assert_queries + assert_queries(1) { Post.first } + + error = assert_raises(Minitest::Assertion) { + assert_queries(2) { Post.first } + } + assert_match(/1 instead of 2 queries/, error.message) + + error = assert_raises(Minitest::Assertion) { + assert_queries(0) { Post.first } + } + assert_match(/1 instead of 0 queries/, error.message) + end + end + + def test_assert_no_queries + assert_no_queries { Post.none } + + error = assert_raises(Minitest::Assertion) { + assert_no_queries { Post.first } + } + assert_match(/1 .* instead of 2/, error.message) + end + end +end diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index 473509331f2..84fa407dbda 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -12,6 +12,8 @@ require "active_support/core_ext/object/try" require "active_support/testing/autorun" require "image_processing/mini_magick" +require "active_record/testing/query_assertions" + require "active_job" ActiveJob::Base.queue_adapter = :test ActiveJob::Base.logger = ActiveSupport::Logger.new(nil) @@ -24,6 +26,7 @@ class ActiveSupport::TestCase self.file_fixture_path = ActiveStorage::FixtureSet.file_fixture_path include ActiveRecord::TestFixtures + include ActiveRecord::Assertions::QueryAssertions self.fixture_paths = [File.expand_path("fixtures", __dir__)] @@ -35,23 +38,6 @@ class ActiveSupport::TestCase ActiveStorage::Current.reset end - def assert_queries(expected_count, matcher: nil, &block) - ActiveRecord::Base.connection.materialize_transactions - - queries = [] - ActiveSupport::Notifications.subscribe("sql.active_record") do |*, payload| - queries << payload[:sql] if %w[ SCHEMA TRANSACTION ].exclude?(payload[:name]) && (matcher.nil? || payload[:sql].match(matcher)) - end - - result = _assert_nothing_raised_or_warn("assert_queries", &block) - assert_equal expected_count, queries.size, "#{queries.size} instead of #{expected_count} queries were executed. Queries: #{queries.join("\n\n")}" - result - end - - def assert_no_queries(&block) - assert_queries(0, &block) - end - private def create_blob(key: nil, data: "Hello world!", filename: "hello.txt", content_type: "text/plain", identify: true, service_name: nil, record: nil) ActiveStorage::Blob.create_and_upload! key: key, io: StringIO.new(data), filename: filename, content_type: content_type, identify: identify, service_name: service_name, record: record diff --git a/railties/lib/rails/test_help.rb b/railties/lib/rails/test_help.rb index e69b6bc6241..f654e52d9ce 100644 --- a/railties/lib/rails/test_help.rb +++ b/railties/lib/rails/test_help.rb @@ -11,15 +11,16 @@ require "action_controller" require "action_controller/test_case" require "action_dispatch/testing/integration" require "rails/generators/test_case" - require "active_support/testing/autorun" require "rails/testing/maintain_test_schema" if defined?(ActiveRecord::Base) + require "active_record/testing/query_assertions" ActiveSupport.on_load(:active_support_test_case) do include ActiveRecord::TestDatabases include ActiveRecord::TestFixtures + include ActiveRecord::Assertions::QueryAssertions self.fixture_paths << "#{Rails.root}/test/fixtures/" self.file_fixture_path = "#{Rails.root}/test/fixtures/files"