8 files changed, 45 insertions, 28 deletions

diff --git a/activerecord/Rakefile b/activerecord/Rakefile
index 2d0d5bd657..fe5f9d1071 100644
--- a/activerecord/Rakefile
+++ b/activerecord/Rakefile
@@ -1,7 +1,7 @@
 require "rake/testtask"
 
-require File.expand_path("test/config", __dir__)
-require File.expand_path("test/support/config", __dir__)
+require_relative "test/config"
+require_relative "test/support/config"
 
 def run_without_aborting(*tasks)
   errors = []
diff --git a/activerecord/bin/test b/activerecord/bin/test
index 3a9547e5c1..ab69f4f603 100755
--- a/activerecord/bin/test
+++ b/activerecord/bin/test
@@ -1,7 +1,7 @@
 #!/usr/bin/env ruby
 
 COMPONENT_ROOT = File.expand_path("..", __dir__)
-require File.expand_path("../tools/test", COMPONENT_ROOT)
+require_relative "../../tools/test"
 
 module Minitest
   def self.plugin_active_record_options(opts, options)
diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb
index 2f32caa257..b9b2acff37 100644
--- a/activerecord/lib/active_record/attribute_methods/primary_key.rb
+++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb
@@ -8,17 +8,14 @@ module ActiveRecord
       # Returns this record's primary key value wrapped in an array if one is
       # available.
       def to_key
-        sync_with_transaction_state
         key = id
         [key] if key
       end
 
       # Returns the primary key value.
       def id
-        if pk = self.class.primary_key
-          sync_with_transaction_state
-          _read_attribute(pk)
-        end
+        sync_with_transaction_state
+        _read_attribute(self.class.primary_key) if self.class.primary_key
       end
 
       # Sets the primary key value.
@@ -57,16 +54,12 @@ module ActiveRecord
         end
 
         module ClassMethods
-          def define_method_attribute(attr_name)
-            super
+          ID_ATTRIBUTE_METHODS = %w(id id= id? id_before_type_cast id_was id_in_database).to_set
 
-            if attr_name == primary_key && attr_name != "id"
-              generated_attribute_methods.send(:alias_method, :id, primary_key)
-            end
+          def instance_method_already_implemented?(method_name)
+            super || primary_key && ID_ATTRIBUTE_METHODS.include?(method_name)
           end
 
-          ID_ATTRIBUTE_METHODS = %w(id id= id? id_before_type_cast id_was id_in_database).to_set
-
           def dangerous_attribute_method?(method_name)
             super && !ID_ATTRIBUTE_METHODS.include?(method_name)
           end
diff --git a/activerecord/test/cases/errors_test.rb b/activerecord/test/cases/errors_test.rb
index 73feb831d0..e90669e0c7 100644
--- a/activerecord/test/cases/errors_test.rb
+++ b/activerecord/test/cases/errors_test.rb
@@ -1,4 +1,4 @@
-require_relative "../cases/helper"
+require "cases/helper"
 
 class ErrorsTest < ActiveRecord::TestCase
   def test_can_be_instantiated_with_no_args
diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb
index 7a80bfb899..596a21dcbc 100644
--- a/activerecord/test/cases/migration/compatibility_test.rb
+++ b/activerecord/test/cases/migration/compatibility_test.rb
@@ -90,6 +90,21 @@ module ActiveRecord
         connection.drop_table :more_testings rescue nil
       end
 
+      def test_timestamps_have_null_constraints_if_not_present_in_migration_of_change_table
+        migration = Class.new(ActiveRecord::Migration[4.2]) {
+          def migrate(x)
+            change_table :testings do |t|
+              t.timestamps
+            end
+          end
+        }.new
+
+        ActiveRecord::Migrator.new(:up, [migration]).migrate
+
+        assert connection.columns(:testings).find { |c| c.name == "created_at" }.null
+        assert connection.columns(:testings).find { |c| c.name == "updated_at" }.null
+      end
+
       def test_timestamps_have_null_constraints_if_not_present_in_migration_for_adding_timestamps_to_existing_table
         migration = Class.new(ActiveRecord::Migration[4.2]) {
           def migrate(x)
diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb
index 5ded619716..200d9e6434 100644
--- a/activerecord/test/cases/primary_keys_test.rb
+++ b/activerecord/test/cases/primary_keys_test.rb
@@ -46,7 +46,7 @@ class PrimaryKeysTest < ActiveRecord::TestCase
     topic = Topic.new
     topic.title = "New Topic"
     assert_nil topic.id
-    assert_nothing_raised { topic.save! }
+    topic.save!
     id = topic.id
 
     topicReloaded = Topic.find(id)
@@ -56,23 +56,30 @@ class PrimaryKeysTest < ActiveRecord::TestCase
   def test_customized_primary_key_auto_assigns_on_save
     Keyboard.delete_all
     keyboard = Keyboard.new(name: "HHKB")
-    assert_nothing_raised { keyboard.save! }
+    keyboard.save!
     assert_equal keyboard.id, Keyboard.find_by_name("HHKB").id
   end
 
   def test_customized_primary_key_can_be_get_before_saving
     keyboard = Keyboard.new
     assert_nil keyboard.id
-    assert_nothing_raised { assert_nil keyboard.key_number }
+    assert_nil keyboard.key_number
   end
 
   def test_customized_string_primary_key_settable_before_save
     subscriber = Subscriber.new
-    assert_nothing_raised { subscriber.id = "webster123" }
+    subscriber.id = "webster123"
     assert_equal "webster123", subscriber.id
     assert_equal "webster123", subscriber.nick
   end
 
+  def test_update_with_non_primary_key_id_column
+    subscriber = Subscriber.first
+    subscriber.update(update_count: 1)
+    subscriber.reload
+    assert_equal 1, subscriber.update_count
+  end
+
   def test_string_key
     subscriber = Subscriber.find(subscribers(:first).nick)
     assert_equal(subscribers(:first).name, subscriber.name)
@@ -83,7 +90,7 @@ class PrimaryKeysTest < ActiveRecord::TestCase
     subscriber.id = "jdoe"
     assert_equal("jdoe", subscriber.id)
     subscriber.name = "John Doe"
-    assert_nothing_raised { subscriber.save! }
+    subscriber.save!
     assert_equal("jdoe", subscriber.id)
 
     subscriberReloaded = Subscriber.find("jdoe")
diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb
index 417c6f4832..b5386ba801 100644
--- a/activerecord/test/cases/schema_dumper_test.rb
+++ b/activerecord/test/cases/schema_dumper_test.rb
@@ -320,7 +320,7 @@ class SchemaDumperTest < ActiveRecord::TestCase
 
   def test_schema_dump_keeps_id_false_when_id_is_false_and_unique_not_null_column_added
     output = standard_dump
-    assert_match %r{create_table "subscribers", id: false}, output
+    assert_match %r{create_table "string_key_objects", id: false}, output
   end
 
   if ActiveRecord::Base.connection.supports_foreign_keys?
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 50f1d9bfe7..8863736943 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -807,16 +807,18 @@ ActiveRecord::Schema.define do
     t.string :sponsorable_type
   end
 
-  create_table :string_key_objects, id: false, primary_key: :id, force: true do |t|
-    t.string     :id
-    t.string     :name
-    t.integer    :lock_version, null: false, default: 0
+  create_table :string_key_objects, id: false, force: true do |t|
+    t.string :id, null: false
+    t.string :name
+    t.integer :lock_version, null: false, default: 0
+    t.index :id, unique: true
   end
 
-  create_table :subscribers, force: true, id: false do |t|
+  create_table :subscribers, force: true do |t|
     t.string :nick, null: false
     t.string :name
-    t.column :books_count, :integer, null: false, default: 0
+    t.integer :books_count, null: false, default: 0
+    t.integer :update_count, null: false, default: 0
     t.index :nick, unique: true
   end