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.

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