TEST_CODEOWNERS/rails
3
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Fix 6.1 change_table setting datetime precision

Browse Source 
While working on #48969, I found that some of the Compatibility test
cases were not working correctly. The tests removed in this commit were
never running the `change_table` migration and so were not actually
testing that `change_table` works correctly. The issue is that the two
migrations created in these tests both have `nil` versions, and so the
Migrator only runs the first one.

This commit refactors the tests so that its easier to test the behavior
of each Migration class version (and I think the rest of the tests
should be updated to use this strategy as well). Additionally, since the
tests are fixed it exposed that `t.change` in a `change_table` is not
behaving as expected so that is fixed as well.
This commit is contained in:
Hartley McGuire 2023-08-18 13:47:57 -04:00
parent ed5af00459
commit 9b07b2d6ca
No known key found for this signature in database
GPG Key ID: E823FC1403858A82
3 changed files with 151 additions and 144 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
activerecord/CHANGELOG.md
View File

@ -1,3 +1,7 @@
* Fix `change_table` setting datetime precision for 6.1 Migrations
*Hartley McGuire*
* Fix change_column setting datetime precision for 6.1 Migrations
*Hartley McGuire*

5
activerecord/lib/active_record/migration/compatibility.rb
View File

@ -209,6 +209,11 @@ def new_column_definition(name, type, **options)
super
end
def change(name, type, index: nil, **options)
options[:precision] ||= nil
super
end
def column(name, type, index: nil, **options)
options[:precision] ||= nil
super

286
activerecord/test/cases/migration/compatibility_test.rb
View File

@ -352,150 +352,6 @@ def migrate(x)
connection.drop_table :more_testings rescue nil
end
def test_datetime_doesnt_set_precision_on_change_table_4_2
create_migration = Class.new(ActiveRecord::Migration[4.2]) {
def migrate(x)
create_table :more_testings do |t|
t.datetime :published_at
end
end
}.new
change_migration = Class.new(ActiveRecord::Migration[4.2]) {
def migrate(x)
change_table :more_testings do |t|
t.datetime :published_at, default: Time.now
end
end
}.new
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
ensure
connection.drop_table :more_testings rescue nil
end
def test_datetime_doesnt_set_precision_on_change_table_5_0
create_migration = Class.new(ActiveRecord::Migration[5.0]) {
def migrate(x)
create_table :more_testings do |t|
t.datetime :published_at
end
end
}.new
change_migration = Class.new(ActiveRecord::Migration[5.0]) {
def migrate(x)
change_table :more_testings do |t|
t.datetime :published_at, default: Time.now
end
end
}.new
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
ensure
connection.drop_table :more_testings rescue nil
end
def test_datetime_doesnt_set_precision_on_change_table_5_1
create_migration = Class.new(ActiveRecord::Migration[5.1]) {
def migrate(x)
create_table :more_testings do |t|
t.datetime :published_at
end
end
}.new
change_migration = Class.new(ActiveRecord::Migration[5.1]) {
def migrate(x)
change_table :more_testings do |t|
t.datetime :published_at, default: Time.now
end
end
}.new
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
ensure
connection.drop_table :more_testings rescue nil
end
def test_datetime_doesnt_set_precision_on_change_table_5_2
create_migration = Class.new(ActiveRecord::Migration[5.2]) {
def migrate(x)
create_table :more_testings do |t|
t.datetime :published_at
end
end
}.new
change_migration = Class.new(ActiveRecord::Migration[5.2]) {
def migrate(x)
change_table :more_testings do |t|
t.datetime :published_at, default: Time.now
end
end
}.new
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
ensure
connection.drop_table :more_testings rescue nil
end
def test_datetime_doesnt_set_precision_on_change_table_6_0
create_migration = Class.new(ActiveRecord::Migration[6.0]) {
def migrate(x)
create_table :more_testings do |t|
t.datetime :published_at
end
end
}.new
change_migration = Class.new(ActiveRecord::Migration[6.0]) {
def migrate(x)
change_table :more_testings do |t|
t.datetime :published_at, default: Time.now
end
end
}.new
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
ensure
connection.drop_table :more_testings rescue nil
end
def test_datetime_doesnt_set_precision_on_change_table_6_1
create_migration = Class.new(ActiveRecord::Migration[6.1]) {
def migrate(x)
create_table :more_testings do |t|
t.datetime :published_at
end
end
}.new
change_migration = Class.new(ActiveRecord::Migration[6.1]) {
def migrate(x)
change_table :more_testings do |t|
t.datetime :published_at, default: Time.now
end
end
}.new
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
ensure
connection.drop_table :more_testings rescue nil
end
def test_datetime_doesnt_set_precision_on_add_column_5_0
migration = Class.new(ActiveRecord::Migration[5.0]) {
def migrate(x)
@ -842,6 +698,148 @@ def precision_implicit_default
end
end
module DefaultPrecisionImplicitTestCases
def test_datetime_doesnt_set_precision_on_change_table
create_migration = Class.new(migration_class) {
def migrate(x)
create_table :more_testings do |t|
t.datetime :published_at
end
end
}.new(nil, 1)
change_migration = Class.new(migration_class) {
def migrate(x)
change_table :more_testings do |t|
t.change :published_at, :datetime, default: Time.now
end
end
}.new(nil, 2)
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
ensure
connection.drop_table :more_testings rescue nil
end
private
def precision_implicit_default
if current_adapter?(:Mysql2Adapter, :TrilogyAdapter)
{ precision: 0 }
else
{ precision: nil }
end
end
end
module DefaultPrecisionSixTestCases
def test_datetime_sets_precision_6_on_change_table
create_migration = Class.new(migration_class) {
def migrate(x)
create_table :more_testings do |t|
t.datetime :published_at
end
end
}.new(nil, 1)
change_migration = Class.new(migration_class) {
def migrate(x)
change_table :more_testings do |t|
t.change :published_at, :datetime, default: Time.now
end
end
}.new(nil, 2)
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
assert connection.column_exists?(:more_testings, :published_at, precision: 6)
ensure
connection.drop_table :more_testings rescue nil
end
end
class BaseCompatibilityTest < ActiveRecord::TestCase
attr_reader :connection
def setup
@connection = ActiveRecord::Base.connection
@schema_migration = @connection.schema_migration
@internal_metadata = @connection.internal_metadata
@verbose_was = ActiveRecord::Migration.verbose
ActiveRecord::Migration.verbose = false
end
def teardown
ActiveRecord::Migration.verbose = @verbose_was
@schema_migration.delete_all_versions rescue nil
end
end
class CompatibilityTest7_0 < BaseCompatibilityTest
include DefaultPrecisionSixTestCases
private
def migration_class
ActiveRecord::Migration[7.0]
end
end
class CompatibilityTest6_1 < BaseCompatibilityTest
include DefaultPrecisionImplicitTestCases
private
def migration_class
ActiveRecord::Migration[6.1]
end
end
class CompatibilityTest6_0 < BaseCompatibilityTest
include DefaultPrecisionImplicitTestCases
private
def migration_class
ActiveRecord::Migration[6.0]
end
end
class CompatibilityTest5_2 < BaseCompatibilityTest
include DefaultPrecisionImplicitTestCases
private
def migration_class
ActiveRecord::Migration[5.2]
end
end
class CompatibilityTest5_1 < BaseCompatibilityTest
include DefaultPrecisionImplicitTestCases
private
def migration_class
ActiveRecord::Migration[5.1]
end
end
class CompatibilityTest5_0 < BaseCompatibilityTest
include DefaultPrecisionImplicitTestCases
private
def migration_class
ActiveRecord::Migration[5.0]
end
end
class CompatibilityTest4_2 < BaseCompatibilityTest
include DefaultPrecisionImplicitTestCases
private
def migration_class
ActiveRecord::Migration[4.2]
end
end
module LegacyPolymorphicReferenceIndexTestCases
attr_reader :connection