Merge branch 'master' into adequaterecord

* master:
  Make AR::Base#touch fire the after_commit and after_rollback callbacks
  Fix test for cache_key + touched
  Revert "methods are defined right after the module_eval, so we don't need to do"
  Revert "Don't remove trailing slash from PATH_INFO for mounted apps"
  Add failing test for #13369
  reset column information after fiddling with `Encoding.default_internal`
  we have `with_env_tz` as global test helper. Remove duplicate.
  isolate class attribute assignment in `migration_test.rb`
  use `teardown` for cleanup, not `setup`.
  tests without transactional fixtures need to cleanup afterwards.
  no need to `return skip` in tests. `skip` is enough.
  methods are defined right after the module_eval, so we don't need to do any line number maths
  Get rid of unused TransactionError constant
  Avoid converting :on option to array twice when defining commit/rollback callbacks
  Unify changelog entries about single quotes [ci skip]
  Use single quotes in generated files

12 files changed, 117 insertions, 54 deletions

diff --git a/activerecord/test/cases/adapters/postgresql/json_test.rb b/activerecord/test/cases/adapters/postgresql/json_test.rb
index 01e7334aad..3daef399d8 100644
--- a/activerecord/test/cases/adapters/postgresql/json_test.rb
+++ b/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
diff --git a/activerecord/test/cases/adapters/postgresql/range_test.rb b/activerecord/test/cases/adapters/postgresql/range_test.rb
index a56b8ac791..5c2d8e1c1d 100644
--- a/activerecord/test/cases/adapters/postgresql/range_test.rb
+++ b/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,
diff --git a/activerecord/test/cases/adapters/postgresql/xml_test.rb b/activerecord/test/cases/adapters/postgresql/xml_test.rb
index bf14b378d8..dd2a727afe 100644
--- a/activerecord/test/cases/adapters/postgresql/xml_test.rb
+++ b/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
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index 517d2674a7..fe5de44409 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/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
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 4bc6002bfe..983bcd9826 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/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
 
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 5125d5df2a..9b575557de 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/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
diff --git a/activerecord/test/cases/integration_test.rb b/activerecord/test/cases/integration_test.rb
index 2e71b1a40d..dfb8a608cb 100644
--- a/activerecord/test/cases/integration_test.rb
+++ b/activerecord/test/cases/integration_test.rb
@@ -3,12 +3,11 @@
 require 'cases/helper'
 require 'models/company'
 require 'models/developer'
-require 'models/car'
-require 'models/bulb'
 require 'models/owner'
+require 'models/pet'
 
 class IntegrationTest < ActiveRecord::TestCase
-  fixtures :companies, :developers, :owners
+  fixtures :companies, :developers, :owners, :pets
 
   def test_to_param_should_return_string
     assert_kind_of String, Client.first.to_param
@@ -91,13 +90,14 @@ class IntegrationTest < ActiveRecord::TestCase
   end
 
   def test_cache_key_changes_when_child_touched
-    car = Car.create
-    Bulb.create(car: car)
+    owner = owners(:blackbeard)
+    pet   = pets(:parrot)
+
+    owner.update_column :updated_at, Time.current
+    key = owner.cache_key
 
-    key = car.cache_key
-    car.bulb.touch
-    car.reload
-    assert_not_equal key, car.cache_key
+    assert pet.touch
+    assert_not_equal key, owner.reload.cache_key
   end
 
   def test_cache_key_format_for_existing_record_with_nil_updated_timestamps
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index 0363bf1048..1bda472d23 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/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.table_name_suffix = '_suffix'
-    Reminder.reset_table_name
+    reminder_class.table_name_prefix = 'prefix_'
+    reminder_class.table_name_suffix = '_suffix'
+    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.table_name_suffix = ''
-    Reminder.reset_table_name
+    reminder_class.table_name_prefix = ''
+    reminder_class.table_name_suffix = ''
+    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)
-    Reminder.reset_table_name
-    assert_equal Reminder.table_name, migration.proper_table_name(Reminder)
+    assert_equal "reminders", migration.proper_table_name(reminder_class)
+    reminder_class.reset_table_name
+    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.table_name_suffix = '_suffix'
-    Reminder.reset_table_name
-    assert_equal "prefix_reminders_suffix", migration.proper_table_name(Reminder)
-    Reminder.table_name_prefix = ''
-    Reminder.table_name_suffix = ''
-    Reminder.reset_table_name
+    reminder_class.table_name_prefix = 'prefix_'
+    reminder_class.table_name_suffix = '_suffix'
+    reminder_class.reset_table_name
+    assert_equal "prefix_reminders_suffix", migration.proper_table_name(reminder_class)
+    reminder_class.table_name_prefix = ''
+    reminder_class.table_name_suffix = ''
+    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')
-      old_tz, ENV['TZ'] = ENV['TZ'], new_tz
-      yield
-    ensure
-      old_tz ? ENV['TZ'] = old_tz : ENV.delete('TZ')
+    # This is needed to isolate class_attribute assignments like `table_name_prefix`
+    # for each test case.
+    def new_isolated_reminder_class
+      Class.new(Reminder) {
+        def self.name; "Reminder"; end
+        def self.base_class; self; end
+      }
     end
 end
 
diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb
index 5566563116..da8ae672fe 100644
--- a/activerecord/test/cases/query_cache_test.rb
+++ b/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
diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb
index 5644a35385..fe0781b7ec 100644
--- a/activerecord/test/cases/transaction_callbacks_test.rb
+++ b/activerecord/test/cases/transaction_callbacks_test.rb
@@ -1,9 +1,11 @@
 require "cases/helper"
+require 'models/owner'
+require 'models/pet'
 require 'models/topic'
 
 class TransactionCallbacksTest < ActiveRecord::TestCase
   self.use_transactional_fixtures = false
-  fixtures :topics
+  fixtures :topics, :owners, :pets
 
   class ReplyWithCallbacks < ActiveRecord::Base
     self.table_name = :topics
@@ -120,6 +122,18 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
     assert_equal [], reply.history
   end
 
+  def test_only_call_after_commit_on_update_after_transaction_commits_for_existing_record_on_touch
+    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
+    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
+    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
+    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
+    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
+    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+
+    @first.touch
+    assert_equal [:commit_on_update], @first.history
+  end
+
   def test_call_after_rollback_after_transaction_rollsback
     @first.after_commit_block{|r| r.history << :after_commit}
     @first.after_rollback_block{|r| r.history << :after_rollback}
@@ -148,6 +162,22 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
     assert_equal [:rollback_on_update], @first.history
   end
 
+  def test_only_call_after_rollback_on_update_after_transaction_rollsback_for_existing_record_on_touch
+    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
+    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
+    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
+    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
+    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
+    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+
+    Topic.transaction do
+      @first.touch
+      raise ActiveRecord::Rollback
+    end
+
+    assert_equal [:rollback_on_update], @first.history
+  end
+
   def test_only_call_after_rollback_on_destroy_after_transaction_rollsback_for_destroyed_record
     @first.after_commit_block(:create){|r| r.history << :commit_on_create}
     @first.after_commit_block(:update){|r| r.history << :commit_on_update}
@@ -279,6 +309,21 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   def test_after_commit_callbacks_should_validate_on_condition
     assert_raise(ArgumentError) { Topic.send(:after_commit, :on => :save) }
   end
+
+  def test_saving_a_record_with_a_belongs_to_that_specifies_touching_the_parent_should_call_callbacks_on_the_parent_object
+    pet   = Pet.first
+    owner = pet.owner
+    flag = false
+
+    owner.on_after_commit do
+      flag = true
+    end
+
+    pet.name = "Fluffy the Third"
+    pet.save
+
+    assert flag
+  end
 end
 
 class CallbacksOnMultipleActionsTest < ActiveRecord::TestCase
diff --git a/activerecord/test/models/car.rb b/activerecord/test/models/car.rb
index c4a15a79e2..db0f93f63b 100644
--- a/activerecord/test/models/car.rb
+++ b/activerecord/test/models/car.rb
@@ -15,7 +15,6 @@ class Car < ActiveRecord::Base
   scope :incl_engines, -> { includes(:engines) }
 
   scope :order_using_new_style,  -> { order('name asc') }
-
 end
 
 class CoolCar < Car
diff --git a/activerecord/test/models/owner.rb b/activerecord/test/models/owner.rb
index 1c7ed4aa3e..cf24502d3a 100644
--- a/activerecord/test/models/owner.rb
+++ b/activerecord/test/models/owner.rb
@@ -2,4 +2,21 @@ class Owner < ActiveRecord::Base
   self.primary_key = :owner_id
   has_many :pets, -> { order 'pets.name desc' }
   has_many :toys, :through => :pets
+
+  after_commit :execute_blocks
+
+  def blocks
+    @blocks ||= []
+  end
+
+  def on_after_commit(&block)
+    blocks << block
+  end
+
+  def execute_blocks
+    blocks.each do |block|
+      block.call(self)
+    end
+    @blocks = []
+  end
 end