Skip to content

Commit

Permalink
Connect to multiple databases in test suite
Browse files Browse the repository at this point in the history
This commit is the first step towards adding support for multiple
databases. It uses transient_record 2.0, which adds support for
multiple databases, and converts the test suite to it.

Multi-database support is going to be added to detectors in subsequent
commits.
  • Loading branch information
gregnavis committed Oct 14, 2023
1 parent 6992ac9 commit 8bf2f1a
Show file tree
Hide file tree
Showing 19 changed files with 422 additions and 345 deletions.
2 changes: 1 addition & 1 deletion active_record_doctor.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ Gem::Specification.new do |s|
s.add_development_dependency "pg", "~> 1.1.4"
s.add_development_dependency "railties", ACTIVE_RECORD_SPEC
s.add_development_dependency "rake", "~> 12.3.3"
s.add_development_dependency "transient_record", "= 1.0.0.rc1"
s.add_development_dependency "transient_record", "= 2.0.0.rc2"

# We don't install rubocop in CI because we test against older Rubies that
# are incompatible with Rubocop.
Expand Down
4 changes: 2 additions & 2 deletions lib/active_record_doctor/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ def mysql?(connection = ActiveRecord::Base.connection)

def expression_indexes_unsupported?(connection = ActiveRecord::Base.connection)
(ActiveRecord::VERSION::STRING < "5.0") ||
# Active Record < 6 is unable to correctly parse expression indexes for MySQL.
(mysql?(connection) && ActiveRecord::VERSION::STRING < "6.0")
# Active Record is unable to correctly parse expression indexes for MySQL.
(mysql?(connection) && ActiveRecord::VERSION::STRING < "7.1")
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/active_record_doctor/detectors/disable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class ActiveRecordDoctor::Detectors::DisableTest < Minitest::Test
# Disabling detectors is implemented in the base class. It's enought to test
# it on a single detector to be reasonably certain it works on all of them.
def test_disabling
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.string :name, null: true
end.define_model do
validates :name, presence: true
Expand Down
40 changes: 20 additions & 20 deletions test/active_record_doctor/detectors/extraneous_indexes_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

class ActiveRecordDoctor::Detectors::ExtraneousIndexesTest < Minitest::Test
def test_index_on_primary_key_is_duplicate
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.index :id
end

Expand All @@ -14,7 +14,7 @@ def test_index_on_primary_key_is_duplicate
def test_partial_index_on_primary_key
skip("MySQL doesn't support partial indexes") if mysql?

create_table(:users) do |t|
Context.create_table(:users) do |t|
t.boolean :admin
t.index :id, where: "admin"
end
Expand All @@ -23,7 +23,7 @@ def test_partial_index_on_primary_key
end

def test_index_on_non_standard_primary_key
create_table(:profiles, primary_key: :user_id) do |t|
Context.create_table(:profiles, primary_key: :user_id) do |t|
t.index :user_id
end

Expand All @@ -33,7 +33,7 @@ def test_index_on_non_standard_primary_key
end

def test_non_unique_version_of_index_is_duplicate
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.string :email
t.index :email, unique: true, name: "unique_index_on_users_email"
end
Expand All @@ -47,7 +47,7 @@ def test_non_unique_version_of_index_is_duplicate
end

def test_single_column_covered_by_unique_and_non_unique_multi_column_is_duplicate
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.string :first_name
t.string :last_name
t.string :email
Expand All @@ -64,7 +64,7 @@ def test_single_column_covered_by_unique_and_non_unique_multi_column_is_duplicat
end

def test_multi_column_covered_by_unique_and_non_unique_multi_column_is_duplicate
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.string :first_name
t.string :last_name
t.string :email
Expand All @@ -83,7 +83,7 @@ def test_multi_column_covered_by_unique_and_non_unique_multi_column_is_duplicate
end

def test_unique_index_with_fewer_columns
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.string :first_name
t.string :last_name
t.index :first_name, unique: true
Expand All @@ -98,7 +98,7 @@ def test_unique_index_with_fewer_columns
def test_expression_index_not_covered_by_multicolumn_index
skip("Expression indexes are not supported") if ActiveRecordDoctor::Utils.expression_indexes_unsupported?

create_table(:users) do |t|
Context.create_table(:users) do |t|
t.string :first_name
t.string :email
t.index "(lower(email))"
Expand All @@ -111,7 +111,7 @@ def test_expression_index_not_covered_by_multicolumn_index
def test_unique_expression_index_not_covered_by_unique_multicolumn_index
skip("Expression indexes are not supported") if ActiveRecordDoctor::Utils.expression_indexes_unsupported?

create_table(:users) do |t|
Context.create_table(:users) do |t|
t.string :first_name
t.string :email
t.index "(lower(email))", unique: true
Expand All @@ -122,7 +122,7 @@ def test_unique_expression_index_not_covered_by_unique_multicolumn_index
end

def test_not_covered_by_different_index_type
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.string :first_name
t.string :last_name
t.index [:last_name, :first_name], using: :btree
Expand All @@ -140,7 +140,7 @@ def test_not_covered_by_different_index_type
def test_not_covered_by_partial_index
skip("MySQL doesn't support partial indexes") if mysql?

create_table(:users) do |t|
Context.create_table(:users) do |t|
t.string :first_name
t.string :last_name
t.boolean :active
Expand All @@ -155,7 +155,7 @@ def test_not_covered_with_different_opclasses
skip("ActiveRecord < 5.2 doesn't support operator classes") if ActiveRecord::VERSION::STRING < "5.2"
skip("MySQL doesn't support operator classes") if mysql?

create_table(:users) do |t|
Context.create_table(:users) do |t|
t.string :first_name
t.string :last_name
t.index [:last_name, :first_name], opclass: :varchar_pattern_ops
Expand All @@ -169,7 +169,7 @@ def test_single_column_covered_by_multi_column_on_materialized_view_is_duplicate
skip("Only PostgreSQL supports materialized views") unless postgresql?

begin
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.string :first_name
t.string :last_name
t.integer :age
Expand Down Expand Up @@ -197,7 +197,7 @@ def test_config_ignore_tables
# ignore_tables into account. We trigger those errors by indexing the
# primary key (the first extraneous index) and then indexing email twice
# (index2... is the other extraneous index).
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.index :id
t.string :email

Expand All @@ -216,7 +216,7 @@ def test_config_ignore_tables
end

def test_config_ignore_tables_regexp
create_table(:users_tmp) do |t|
Context.create_table(:users_tmp) do |t|
t.index :id
end

Expand All @@ -231,7 +231,7 @@ def test_config_ignore_tables_regexp
end

def test_config_ignore_tables_string_ignores_exact_match
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.index :id
end

Expand All @@ -248,7 +248,7 @@ def test_config_ignore_tables_string_ignores_exact_match
end

def test_config_global_ignore_tables
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.index :id
t.string :email

Expand All @@ -266,7 +266,7 @@ def test_config_global_ignore_tables
end

def test_config_global_ignore_indexes
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.index :id
t.string :email

Expand All @@ -288,7 +288,7 @@ def test_config_global_ignore_indexes
end

def test_config_detector_ignore_indexes
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.index :id
t.string :email
t.string :api_key
Expand All @@ -308,7 +308,7 @@ def test_config_detector_ignore_indexes
end

def test_config_detector_ignore_indexes_regexp
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.index :id
t.string :email
t.string :api_key
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

class ActiveRecordDoctor::Detectors::IncorrectBooleanPresenceValidationTest < Minitest::Test
def test_presence_true_is_reported_on_boolean_only
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.string :email, null: false
t.boolean :active, null: false
end.define_model do
Expand All @@ -12,13 +12,20 @@ def test_presence_true_is_reported_on_boolean_only
validates :email, :active, presence: true
end

SecondaryContext.create_table(:subscriptions) do |t|
t.boolean :active, null: false
end.define_model do
validates :active, presence: true
end

assert_problems(<<~OUTPUT)
replace the `presence` validator on TransientRecord::Models::User.active with `inclusion` - `presence` can't be used on booleans
replace the `presence` validator on Context::User.active with `inclusion` - `presence` can't be used on booleans
replace the `presence` validator on SecondaryContext::Subscription.active with `inclusion` - `presence` can't be used on booleans
OUTPUT
end

def test_inclusion_is_not_reported
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.boolean :active, null: false
end.define_model do
validates :active, inclusion: { in: [true, false] }
Expand All @@ -28,13 +35,31 @@ def test_inclusion_is_not_reported
end

def test_models_with_non_existent_tables_are_skipped
define_model(:User)
Context.define_model(:User)

refute_problems
end

def test_config_ignore_databases
SecondaryContext.create_table(:users) do |t|
t.string :email, null: false
t.boolean :active, null: false
end.define_model do
validates :email, :active, presence: true
end

config_file(<<-CONFIG)
ActiveRecordDoctor.configure do |config|
config.detector :incorrect_boolean_presence_validation,
ignore_databases: [:secondary]
end
CONFIG

refute_problems
end

def test_config_ignore_models
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.boolean :active, null: false
end.define_model do
validates :active, presence: true
Expand All @@ -43,15 +68,15 @@ def test_config_ignore_models
config_file(<<-CONFIG)
ActiveRecordDoctor.configure do |config|
config.detector :incorrect_boolean_presence_validation,
ignore_models: ["TransientRecord::Models::User"]
ignore_models: ["Context::User"]
end
CONFIG

refute_problems
end

def test_config_ignore_models_regexp
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.boolean :active, null: false
end.define_model do
validates :active, presence: true
Expand All @@ -68,23 +93,23 @@ def test_config_ignore_models_regexp
end

def test_global_ignore_models
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.boolean :active, null: false
end.define_model do
validates :active, presence: true
end

config_file(<<-CONFIG)
ActiveRecordDoctor.configure do |config|
config.global :ignore_models, ["TransientRecord::Models::User"]
config.global :ignore_models, ["Context::User"]
end
CONFIG

refute_problems
end

def test_config_ignore_attributes
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.boolean :active, null: false
end.define_model do
validates :active, presence: true
Expand All @@ -93,15 +118,15 @@ def test_config_ignore_attributes
config_file(<<-CONFIG)
ActiveRecordDoctor.configure do |config|
config.detector :incorrect_boolean_presence_validation,
ignore_attributes: ["TransientRecord::Models::User.active"]
ignore_attributes: ["Context::User.active"]
end
CONFIG

refute_problems
end

def test_config_ignore_attributes_regexp
create_table(:users) do |t|
Context.create_table(:users) do |t|
t.boolean :active_old, null: false
end.define_model do
validates :active_old, presence: true
Expand Down
Loading

0 comments on commit 8bf2f1a

Please sign in to comment.