mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
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.
This commit is contained in:
parent
41d63710f2
commit
88d6ae304e
5 changed files with 36 additions and 24 deletions
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in a new issue