From f644f7d35bb84d3ed826b89ee20859d27d769037 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20D=C3=ADaz?= Date: Sat, 12 Jun 2021 17:26:59 -0500 Subject: [PATCH] Extract methods `assert_queries` and `assert_no_queries` Both methods are defined in multiple parts of the framework. It would be useful to put them in a proper place, so that repetition is avoided. I chose the implementation from `ActiveRecord` because it's a bit more complete with the `SQLCounter` class, and also because other parts depend on it. --- actiontext/test/test_helper.rb | 19 ++------ .../test/activerecord/relation_cache_test.rb | 16 ++----- .../active_record/testing/query_assertions.rb | 47 +++++++++++++++++++ activerecord/test/cases/dirty_test.rb | 2 +- activerecord/test/cases/test_case.rb | 41 +--------------- activestorage/test/test_helper.rb | 20 ++------ 6 files changed, 59 insertions(+), 86 deletions(-) create mode 100644 activerecord/lib/active_record/testing/query_assertions.rb diff --git a/actiontext/test/test_helper.rb b/actiontext/test/test_helper.rb index 9aca998e1b4..975b8cb89b9 100644 --- a/actiontext/test/test_helper.rb +++ b/actiontext/test/test_helper.rb @@ -10,6 +10,8 @@ require "rails/test_help" require "rails/test_unit/reporter" Rails::TestUnitReporter.executable = "bin/test" +require "active_record/testing/query_assertions" + # Disable available locale checks to allow to add locale after initialized. I18n.enforce_available_locales = false @@ -22,22 +24,7 @@ if ActiveSupport::TestCase.respond_to?(:fixture_path=) end class ActiveSupport::TestCase - def assert_queries(expected_count) - 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 - - yield.tap do - assert_equal expected_count, queries.size, "#{queries.size} instead of #{expected_count} queries were executed. #{queries.inspect}" - end - end - - def assert_no_queries(&block) - assert_queries(0, &block) - end + include ActiveRecord::Testing::QueryAssertions private def create_file_blob(filename:, content_type:, metadata: nil) diff --git a/actionview/test/activerecord/relation_cache_test.rb b/actionview/test/activerecord/relation_cache_test.rb index 26f3bfbcbc8..259707edd5e 100644 --- a/actionview/test/activerecord/relation_cache_test.rb +++ b/actionview/test/activerecord/relation_cache_test.rb @@ -1,8 +1,11 @@ # frozen_string_literal: true require "active_record_unit" +require "active_record/testing/query_assertions" class RelationCacheTest < ActionView::TestCase + include ActiveRecord::Testing::QueryAssertions + tests ActionView::Helpers::CacheHelper def setup @@ -24,17 +27,4 @@ class RelationCacheTest < ActionView::TestCase end def view_cache_dependencies; []; end - - def assert_queries(num) - 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 = yield - assert_equal num, count, "#{count} instead of #{num} queries were executed." - result - end end 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..bcba661dfee --- /dev/null +++ b/activerecord/lib/active_record/testing/query_assertions.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module ActiveRecord + module Testing + module QueryAssertions # :nodoc: + def assert_queries(expected_count = 1, options = {}) + ignore_none = options.fetch(:ignore_none) { expected_count == :any } + ActiveRecord::Base.connection.materialize_transactions + SQLCounter.clear_log + result = yield + the_log = ignore_none ? SQLCounter.log_all : SQLCounter.log + + if expected_count == :any + assert_operator the_log.size, :>=, 1, "1 or more queries expected, but none were executed." + else + message = "#{the_log.size} instead of #{expected_count} queries were executed.#{the_log.size == 0 ? '' : "\nQueries:\n#{the_log.join("\n")}"}" + assert_equal expected_count, the_log.size, message + end + + result + end + + def assert_no_queries(&block) + assert_queries(0, &block) + end + + class SQLCounter # :nodoc: + class << self + attr_accessor :ignored_sql, :log, :log_all + def clear_log; self.log = []; self.log_all = []; end + end + + clear_log + + def call(*, values) + return if values[:cached] + + sql = values[:sql] + self.class.log_all << sql + self.class.log << sql unless ["SCHEMA", "TRANSACTION"].include? values[:name] + end + end + + ActiveSupport::Notifications.subscribe("sql.active_record", SQLCounter.new) + end + end +end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index 0f84e39e9f4..91a1672917b 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -649,7 +649,7 @@ class DirtyTest < ActiveRecord::TestCase jon = Person.create! first_name: "Jon" end - assert ActiveRecord::SQLCounter.log_all.none? { |sql| sql.include?("followers_count") } + assert SQLCounter.log_all.none? { |sql| sql.include?("followers_count") } jon.reload assert_equal "Jon", jon.first_name diff --git a/activerecord/test/cases/test_case.rb b/activerecord/test/cases/test_case.rb index 5bb101c6741..d359d4180b4 100644 --- a/activerecord/test/cases/test_case.rb +++ b/activerecord/test/cases/test_case.rb @@ -4,6 +4,7 @@ require "active_support" require "active_support/testing/autorun" require "active_support/testing/method_call_assertions" require "active_support/testing/stream" +require "active_record/testing/query_assertions" require "active_record/fixtures" require "cases/validations_repair_helper" @@ -14,6 +15,7 @@ module ActiveRecord # Defines some test assertions to test against SQL queries. class TestCase < ActiveSupport::TestCase #:nodoc: include ActiveSupport::Testing::MethodCallAssertions + include ActiveRecord::Testing::QueryAssertions include ActiveSupport::Testing::Stream include ActiveRecord::TestFixtures include ActiveRecord::ValidationsRepairHelper @@ -47,26 +49,6 @@ module ActiveRecord assert failed_patterns.empty?, "Query pattern(s) #{failed_patterns.map(&:inspect).join(', ')} not found.#{SQLCounter.log.size == 0 ? '' : "\nQueries:\n#{SQLCounter.log.join("\n")}"}" end - def assert_queries(num = 1, options = {}) - ignore_none = options.fetch(:ignore_none) { num == :any } - ActiveRecord::Base.connection.materialize_transactions - SQLCounter.clear_log - x = yield - the_log = ignore_none ? SQLCounter.log_all : SQLCounter.log - if num == :any - assert_operator the_log.size, :>=, 1, "1 or more queries expected, but none were executed." - else - mesg = "#{the_log.size} instead of #{num} queries were executed.#{the_log.size == 0 ? '' : "\nQueries:\n#{the_log.join("\n")}"}" - assert_equal num, the_log.size, mesg - end - x - end - - def assert_no_queries(options = {}, &block) - options.reverse_merge! ignore_none: true - assert_queries(0, options, &block) - end - def assert_column(model, column_name, msg = nil) assert has_column?(model, column_name), msg end @@ -135,23 +117,4 @@ module ActiveRecord super if current_adapter?(:SQLite3Adapter) end end - - class SQLCounter - class << self - attr_accessor :ignored_sql, :log, :log_all - def clear_log; self.log = []; self.log_all = []; end - end - - clear_log - - def call(name, start, finish, message_id, values) - return if values[:cached] - - sql = values[:sql] - self.class.log_all << sql - self.class.log << sql unless ["SCHEMA", "TRANSACTION"].include? values[:name] - end - end - - ActiveSupport::Notifications.subscribe("sql.active_record", SQLCounter.new) end diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index 41f97e9dfa8..78b03301d1d 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -9,6 +9,7 @@ require "active_support/test_case" require "active_support/core_ext/object/try" require "active_support/testing/autorun" require "active_support/configuration_file" +require "active_record/testing/query_assertions" require "active_storage/service/mirror_service" require "image_processing/mini_magick" @@ -46,6 +47,8 @@ ActiveStorage.verifier = ActiveSupport::MessageVerifier.new("Testing") ActiveStorage::FixtureSet.file_fixture_path = File.expand_path("fixtures/files", __dir__) class ActiveSupport::TestCase + include ActiveRecord::Testing::QueryAssertions + self.file_fixture_path = ActiveStorage::FixtureSet.file_fixture_path include ActiveRecord::TestFixtures @@ -60,23 +63,6 @@ class ActiveSupport::TestCase ActiveStorage::Current.reset end - def assert_queries(expected_count) - 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 - - yield.tap do - assert_equal expected_count, queries.size, "#{queries.size} instead of #{expected_count} queries were executed. #{queries.inspect}" - end - 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