Merge branch 'fix_order_dependent_tests_in_active_record_suite'

This gets the whole Active Record suite working without calling
`self.i_suck_and_my_tests_are_order_dependent!` on `ActiveSupport::TestCase`.

Note that the test files themselves still run in order. This means
that only order dependencies within a single test file were addressed.

activerecord/test/cases/adapters/postgresql/json_test.rb

@@ -21,7 +21,7 @@ class PostgresqlJSONTest < ActiveRecord::TestCase
         end
       end
     rescue ActiveRecord::StatementInvalid
-      return skip "do not test on PG without json"
+      skip "do not test on PG without json"
     end
     @column = JsonDataType.columns.find { |c| c.name == 'payload' }
   end

activerecord/test/cases/adapters/postgresql/range_test.rb

@@ -26,7 +26,7 @@ if ActiveRecord::Base.connection.supports_ranges?
           end
         end
       rescue ActiveRecord::StatementInvalid
-        return skip "do not test on PG without range"
+        skip "do not test on PG without range"
       end
 
       insert_range(id: 101,

activerecord/test/cases/adapters/postgresql/xml_test.rb

@@ -18,7 +18,7 @@ class PostgresqlXMLTest < ActiveRecord::TestCase
         end
       end
     rescue ActiveRecord::StatementInvalid
-      return skip "do not test on PG without xml"
+      skip "do not test on PG without xml"
     end
     @column = XmlDataType.columns.find { |c| c.name == 'payload' }
   end

activerecord/test/cases/autosave_association_test.rb

@@ -574,6 +574,13 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
     @ship = @pirate.create_ship(:name => 'Nights Dirty Lightning')
   end
 
+  def teardown
+    # We are running without transactional fixtures and need to cleanup.
+    Bird.delete_all
+    @ship.delete
+    @pirate.delete
+  end
+
   # reload
   def test_a_marked_for_destruction_record_should_not_be_be_marked_after_reload
     @pirate.mark_for_destruction

activerecord/test/cases/base_test.rb

@@ -626,6 +626,7 @@ class BasicsTest < ActiveRecord::TestCase
       assert_equal ["EUC-JP"], Weird.columns.map {|c| c.name.encoding.name }.uniq
     ensure
       silence_warnings { Encoding.default_internal = old_default_internal }
+      Weird.reset_column_information
     end
   end
 
@@ -1129,7 +1130,7 @@ class BasicsTest < ActiveRecord::TestCase
     k = Class.new(ak)
     k.table_name = "projects"
     orig_name = k.sequence_name
-    return skip "sequences not supported by db" unless orig_name
+    skip "sequences not supported by db" unless orig_name
     assert_equal k.reset_sequence_name, orig_name
   end
 

activerecord/test/cases/finder_test.rb

@@ -891,11 +891,4 @@ class FinderTest < ActiveRecord::TestCase
         ActiveRecord::Base.send(:replace_bind_variables, statement, vars)
       end
     end
-
-    def with_env_tz(new_tz = 'US/Eastern')
-      old_tz, ENV['TZ'] = ENV['TZ'], new_tz
-      yield
-    ensure
-      old_tz ? ENV['TZ'] = old_tz : ENV.delete('TZ')
-    end
 end

activerecord/test/cases/migration_test.rb

@@ -328,6 +328,7 @@ class MigrationTest < ActiveRecord::TestCase
   end
 
   def test_proper_table_name_on_migrator
+    reminder_class = new_isolated_reminder_class
     assert_deprecated do
       assert_equal "table", ActiveRecord::Migrator.proper_table_name('table')
     end
@@ -335,30 +336,30 @@ class MigrationTest < ActiveRecord::TestCase
       assert_equal "table", ActiveRecord::Migrator.proper_table_name(:table)
     end
     assert_deprecated do
-      assert_equal "reminders", ActiveRecord::Migrator.proper_table_name(Reminder)
+      assert_equal "reminders", ActiveRecord::Migrator.proper_table_name(reminder_class)
     end
-    Reminder.reset_table_name
+    reminder_class.reset_table_name
     assert_deprecated do
-      assert_equal Reminder.table_name, ActiveRecord::Migrator.proper_table_name(Reminder)
+      assert_equal reminder_class.table_name, ActiveRecord::Migrator.proper_table_name(reminder_class)
     end
 
     # Use the model's own prefix/suffix if a model is given
     ActiveRecord::Base.table_name_prefix = "ARprefix_"
     ActiveRecord::Base.table_name_suffix = "_ARsuffix"
-    Reminder.table_name_prefix = 'prefix_'
+    reminder_class.table_name_prefix = 'prefix_'
-    Reminder.table_name_suffix = '_suffix'
+    reminder_class.table_name_suffix = '_suffix'
-    Reminder.reset_table_name
+    reminder_class.reset_table_name
     assert_deprecated do
-      assert_equal "prefix_reminders_suffix", ActiveRecord::Migrator.proper_table_name(Reminder)
+      assert_equal "prefix_reminders_suffix", ActiveRecord::Migrator.proper_table_name(reminder_class)
     end
-    Reminder.table_name_prefix = ''
+    reminder_class.table_name_prefix = ''
-    Reminder.table_name_suffix = ''
+    reminder_class.table_name_suffix = ''
-    Reminder.reset_table_name
+    reminder_class.reset_table_name
 
     # Use AR::Base's prefix/suffix if string or symbol is given
     ActiveRecord::Base.table_name_prefix = "prefix_"
     ActiveRecord::Base.table_name_suffix = "_suffix"
-    Reminder.reset_table_name
+    reminder_class.reset_table_name
     assert_deprecated do
       assert_equal "prefix_table_suffix", ActiveRecord::Migrator.proper_table_name('table')
     end
@@ -368,28 +369,29 @@ class MigrationTest < ActiveRecord::TestCase
   end
 
   def test_proper_table_name_on_migration
+    reminder_class = new_isolated_reminder_class
     migration = ActiveRecord::Migration.new
     assert_equal "table", migration.proper_table_name('table')
     assert_equal "table", migration.proper_table_name(:table)
-    assert_equal "reminders", migration.proper_table_name(Reminder)
+    assert_equal "reminders", migration.proper_table_name(reminder_class)
-    Reminder.reset_table_name
+    reminder_class.reset_table_name
-    assert_equal Reminder.table_name, migration.proper_table_name(Reminder)
+    assert_equal reminder_class.table_name, migration.proper_table_name(reminder_class)
 
     # Use the model's own prefix/suffix if a model is given
     ActiveRecord::Base.table_name_prefix = "ARprefix_"
     ActiveRecord::Base.table_name_suffix = "_ARsuffix"
-    Reminder.table_name_prefix = 'prefix_'
+    reminder_class.table_name_prefix = 'prefix_'
-    Reminder.table_name_suffix = '_suffix'
+    reminder_class.table_name_suffix = '_suffix'
-    Reminder.reset_table_name
+    reminder_class.reset_table_name
-    assert_equal "prefix_reminders_suffix", migration.proper_table_name(Reminder)
+    assert_equal "prefix_reminders_suffix", migration.proper_table_name(reminder_class)
-    Reminder.table_name_prefix = ''
+    reminder_class.table_name_prefix = ''
-    Reminder.table_name_suffix = ''
+    reminder_class.table_name_suffix = ''
-    Reminder.reset_table_name
+    reminder_class.reset_table_name
 
     # Use AR::Base's prefix/suffix if string or symbol is given
     ActiveRecord::Base.table_name_prefix = "prefix_"
     ActiveRecord::Base.table_name_suffix = "_suffix"
-    Reminder.reset_table_name
+    reminder_class.reset_table_name
     assert_equal "prefix_table_suffix", migration.proper_table_name('table', migration.table_name_options)
     assert_equal "prefix_table_suffix", migration.proper_table_name(:table, migration.table_name_options)
   end
@@ -532,11 +534,13 @@ class MigrationTest < ActiveRecord::TestCase
   end
 
   protected
-    def with_env_tz(new_tz = 'US/Eastern')
+    # This is needed to isolate class_attribute assignments like `table_name_prefix`
-      old_tz, ENV['TZ'] = ENV['TZ'], new_tz
+    # for each test case.
-      yield
+    def new_isolated_reminder_class
-    ensure
+      Class.new(Reminder) {
-      old_tz ? ENV['TZ'] = old_tz : ENV.delete('TZ')
+        def self.name; "Reminder"; end
+        def self.base_class; self; end
+      }
     end
 end
 

activerecord/test/cases/query_cache_test.rb

@@ -8,7 +8,7 @@ require 'rack'
 class QueryCacheTest < ActiveRecord::TestCase
   fixtures :tasks, :topics, :categories, :posts, :categories_posts
 
-  def setup
+  teardown do
     Task.connection.clear_query_cache
     ActiveRecord::Base.connection.disable_query_cache!
   end
@@ -214,7 +214,7 @@ class QueryCacheExpiryTest < ActiveRecord::TestCase
     Post.find(1)
 
     # change the column definition
-    Post.connection.change_column :posts, :title, :string, :limit => 80
+    Post.connection.change_column :posts, :title, :string, limit: 80
     assert_nothing_raised { Post.find(1) }
 
     # restore the old definition
@@ -241,7 +241,6 @@ class QueryCacheExpiryTest < ActiveRecord::TestCase
 
   def test_update
     Task.connection.expects(:clear_query_cache).times(2)
-
     Task.cache do
       task = Task.find(1)
       task.starting = Time.now.utc
@@ -251,7 +250,6 @@ class QueryCacheExpiryTest < ActiveRecord::TestCase
 
   def test_destroy
     Task.connection.expects(:clear_query_cache).times(2)
-
     Task.cache do
       Task.find(1).destroy
     end
@@ -259,7 +257,6 @@ class QueryCacheExpiryTest < ActiveRecord::TestCase
 
   def test_insert
     ActiveRecord::Base.connection.expects(:clear_query_cache).times(2)
-
     Task.cache do
       Task.create!
     end