From 88d6ae304e335bc3e6d34f053ef8f3324faa6c1b Mon Sep 17 00:00:00 2001
From: Steve Jorgensen <stevej@renewfund.com>
Date: Fri, 8 Jun 2012 11:10:22 -0700
Subject: Fix fragile #assert_queries implementation and usages.
Several tests that passed when run in the order they are loaded
by rake test were failing when run in different sequences due to
problems with the implementation of assert_queries and
assert_no_queries as well as incorrect assumptions made about
how many queries might be executed by a database adapter in
various cases.
---
activerecord/lib/active_record/test_case.rb | 50 ++++++++++++++--------
activerecord/test/cases/associations/eager_test.rb | 2 -
.../has_and_belongs_to_many_associations_test.rb | 7 ++-
activerecord/test/cases/migration_test.rb | 2 +-
activerecord/test/cases/named_scope_test.rb | 1 -
5 files changed, 37 insertions(+), 25 deletions(-)
diff --git a/activerecord/lib/active_record/test_case.rb b/activerecord/lib/active_record/test_case.rb
index fcaa4b74a6..c7a6c37d50 100644
--- a/activerecord/lib/active_record/test_case.rb
+++ b/activerecord/lib/active_record/test_case.rb
@@ -8,7 +8,7 @@ module ActiveRecord
# Defines some test assertions to test against SQL queries.
class TestCase < ActiveSupport::TestCase #:nodoc:
def teardown
- SQLCounter.log.clear
+ SQLCounter.clear_log
end
def assert_date_from_db(expected, actual, message = nil)
@@ -22,47 +22,57 @@ module ActiveRecord
end
def assert_sql(*patterns_to_match)
- SQLCounter.log = []
+ SQLCounter.clear_log
yield
- SQLCounter.log
+ SQLCounter.log_all
ensure
failed_patterns = []
patterns_to_match.each do |pattern|
- failed_patterns << pattern unless SQLCounter.log.any?{ |sql| pattern === sql }
+ failed_patterns << pattern unless SQLCounter.log_all.any?{ |sql| pattern === sql }
end
assert failed_patterns.empty?, "Query pattern(s) #{failed_patterns.map{ |p| p.inspect }.join(', ')} not found.#{SQLCounter.log.size == 0 ? '' : "\nQueries:\n#{SQLCounter.log.join("\n")}"}"
end
- def assert_queries(num = 1)
- SQLCounter.log = []
+ def assert_queries(num = 1, options = {})
+ ignore_none = options.fetch(:ignore_none) { num == :any }
+ SQLCounter.clear_log
yield
ensure
- assert_equal num, SQLCounter.log.size, "#{SQLCounter.log.size} instead of #{num} queries were executed.#{SQLCounter.log.size == 0 ? '' : "\nQueries:\n#{SQLCounter.log.join("\n")}"}"
+ 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
end
def assert_no_queries(&block)
- prev_ignored_sql = SQLCounter.ignored_sql
- SQLCounter.ignored_sql = []
- assert_queries(0, &block)
- ensure
- SQLCounter.ignored_sql = prev_ignored_sql
+ assert_queries(0, :ignore_none => true, &block)
end
end
class SQLCounter
class << self
- attr_accessor :ignored_sql, :log
+ attr_accessor :ignored_sql, :log, :log_all
+ def clear_log; self.log = []; self.log_all = []; end
end
- self.log = []
+ self.clear_log
self.ignored_sql = [/^PRAGMA (?!(table_info))/, /^SELECT currval/, /^SELECT CAST/, /^SELECT @@IDENTITY/, /^SELECT @@ROWCOUNT/, /^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/, /^SHOW max_identifier_length/, /^BEGIN/, /^COMMIT/]
# FIXME: this needs to be refactored so specific database can add their own
- # ignored SQL. This ignored SQL is for Oracle.
- ignored_sql.concat [/^select .*nextval/i, /^SAVEPOINT/, /^ROLLBACK TO/, /^\s*select .* from all_triggers/im]
-
+ # ignored SQL, or better yet, use a different notification for the queries
+ # instead examining the SQL content.
+ oracle_ignored = [/^select .*nextval/i, /^SAVEPOINT/, /^ROLLBACK TO/, /^\s*select .* from all_triggers/im]
+ mysql_ignored = [/^SHOW TABLES/i, /^SHOW FULL FIELDS/]
+ postgresql_ignored = [/^\s*select\b.*\bfrom\b.*pg_namespace\b/im, /^\s*select\b.*\battname\b.*\bfrom\b.*\bpg_attribute\b/im]
+
+ [oracle_ignored, mysql_ignored, postgresql_ignored].each do |db_ignored_sql|
+ ignored_sql.concat db_ignored_sql
+ end
attr_reader :ignore
@@ -75,8 +85,10 @@ module ActiveRecord
# FIXME: this seems bad. we should probably have a better way to indicate
# the query was cached
- return if 'CACHE' == values[:name] || ignore =~ sql
- self.class.log << sql
+ return if 'CACHE' == values[:name]
+
+ self.class.log_all << sql
+ self.class.log << sql unless ignore =~ sql
end
end
diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb
index 08467900f9..76d3eb8946 100644
--- a/activerecord/test/cases/associations/eager_test.rb
+++ b/activerecord/test/cases/associations/eager_test.rb
@@ -1010,8 +1010,6 @@ class EagerAssociationTest < ActiveRecord::TestCase
end
def test_eager_loading_with_conditions_on_join_model_preloads
- Author.columns
-
authors = assert_queries(2) do
Author.scoped(:includes => :author_address, :joins => :comments, :where => "posts.title like 'Welcome%'").all
end
diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index ed1caa2ef5..9d693bae0c 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -817,11 +817,14 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
# clear cache possibly created by other tests
david.projects.reset_column_information
- assert_queries(1) { david.projects.columns; david.projects.columns }
+ assert_queries(:any) { david.projects.columns }
+ assert_no_queries { david.projects.columns }
## and again to verify that reset_column_information clears the cache correctly
david.projects.reset_column_information
- assert_queries(1) { david.projects.columns; david.projects.columns }
+
+ assert_queries(:any) { david.projects.columns }
+ assert_no_queries { david.projects.columns }
end
def test_attributes_are_being_set_when_initialized_from_habm_association_with_where_clause
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index 5d1bad0d54..fad144ea23 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -523,7 +523,7 @@ if ActiveRecord::Base.connection.supports_bulk_alter?
# One query for columns (delete_me table)
# One query for primary key (delete_me table)
# One query to do the bulk change
- assert_queries(3) do
+ assert_queries(3, :ignore_none => true) do
with_bulk_change_table do |t|
t.change :name, :string, :default => 'NONAME'
t.change :birthdate, :datetime
diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb
index bf825c002a..2762afc921 100644
--- a/activerecord/test/cases/named_scope_test.rb
+++ b/activerecord/test/cases/named_scope_test.rb
@@ -19,7 +19,6 @@ class NamedScopeTest < ActiveRecord::TestCase
end
def test_found_items_are_cached
- Topic.columns
all_posts = Topic.base
assert_queries(1) do
--
cgit v1.2.3