Skip to content

Commit

Permalink
Ensure multiple anonymous modules are not included into Topic in tests
Browse files Browse the repository at this point in the history
Each acceptance validator applied to a model class includes an instance
of a module builder (LazilyDefineAttributes) into that class. In tests,
if the original model class is not subclassed, these modules pile up and
cannot be removed, potentially leading to flakey specs and false
positive/negatives.

To avoid this, always use subclasses in tests whose names (constants)
can be removed when the test is done.
  • Loading branch information
shioyama committed Apr 14, 2019
1 parent 98d4a68 commit 2e27801
Showing 1 changed file with 32 additions and 18 deletions.
50 changes: 32 additions & 18 deletions activemodel/test/cases/validations/acceptance_validation_test.rb
Expand Up @@ -7,21 +7,23 @@
require "models/person"

class AcceptanceValidationTest < ActiveModel::TestCase
def teardown
Topic.clear_validators!
teardown do
self.class.send(:remove_const, :TestClass)
end

def test_terms_of_service_agreement_no_acceptance
Topic.validates_acceptance_of(:terms_of_service)
klass = define_test_class(Topic)
klass.validates_acceptance_of(:terms_of_service)

t = Topic.new("title" => "We should not be confirmed")
t = klass.new("title" => "We should not be confirmed")
assert_predicate t, :valid?
end

def test_terms_of_service_agreement
Topic.validates_acceptance_of(:terms_of_service)
klass = define_test_class(Topic)
klass.validates_acceptance_of(:terms_of_service)

t = Topic.new("title" => "We should be confirmed", "terms_of_service" => "")
t = klass.new("title" => "We should be confirmed", "terms_of_service" => "")
assert_predicate t, :invalid?
assert_equal ["must be accepted"], t.errors[:terms_of_service]

Expand All @@ -30,9 +32,10 @@ def test_terms_of_service_agreement
end

def test_eula
Topic.validates_acceptance_of(:eula, message: "must be abided")
klass = define_test_class(Topic)
klass.validates_acceptance_of(:eula, message: "must be abided")

t = Topic.new("title" => "We should be confirmed", "eula" => "")
t = klass.new("title" => "We should be confirmed", "eula" => "")
assert_predicate t, :invalid?
assert_equal ["must be abided"], t.errors[:eula]

Expand All @@ -41,9 +44,10 @@ def test_eula
end

def test_terms_of_service_agreement_with_accept_value
Topic.validates_acceptance_of(:terms_of_service, accept: "I agree.")
klass = define_test_class(Topic)
klass.validates_acceptance_of(:terms_of_service, accept: "I agree.")

t = Topic.new("title" => "We should be confirmed", "terms_of_service" => "")
t = klass.new("title" => "We should be confirmed", "terms_of_service" => "")
assert_predicate t, :invalid?
assert_equal ["must be accepted"], t.errors[:terms_of_service]

Expand All @@ -52,9 +56,10 @@ def test_terms_of_service_agreement_with_accept_value
end

def test_terms_of_service_agreement_with_multiple_accept_values
Topic.validates_acceptance_of(:terms_of_service, accept: [1, "I concur."])
klass = define_test_class(Topic)
klass.validates_acceptance_of(:terms_of_service, accept: [1, "I concur."])

t = Topic.new("title" => "We should be confirmed", "terms_of_service" => "")
t = klass.new("title" => "We should be confirmed", "terms_of_service" => "")
assert_predicate t, :invalid?
assert_equal ["must be accepted"], t.errors[:terms_of_service]

Expand All @@ -66,23 +71,32 @@ def test_terms_of_service_agreement_with_multiple_accept_values
end

def test_validates_acceptance_of_for_ruby_class
Person.validates_acceptance_of :karma
klass = define_test_class(Person)
klass.validates_acceptance_of :karma

p = Person.new
p = klass.new
p.karma = ""

assert_predicate p, :invalid?
assert_equal ["must be accepted"], p.errors[:karma]

p.karma = "1"
assert_predicate p, :valid?
ensure
Person.clear_validators!
end

def test_validates_acceptance_of_true
Topic.validates_acceptance_of(:terms_of_service)
klass = define_test_class(Topic)
klass.validates_acceptance_of(:terms_of_service)

assert_predicate Topic.new(terms_of_service: true), :valid?
assert_predicate klass.new(terms_of_service: true), :valid?
end

private

# Acceptance validator includes anonymous module into class, which cannot
# be cleared, so to avoid multiple inclusions we use a named subclass which
# we can remove in teardown.
def define_test_class(parent)
self.class.const_set(:TestClass, Class.new(parent))
end
end

0 comments on commit 2e27801

Please sign in to comment.