Skip to content

Commit

Permalink
Add deprecator-only overload for assert_deprecated
Browse files Browse the repository at this point in the history
This commits adds a deprecator-only overload for `assert_deprecated`.
For example, `assert_deprecated(/./, deprecator)` can now be written as
`assert_deprecated(deprecator)`.

This is part of an initiative to move away from the top-level
`ActiveSupport::Deprecation` API, towards per-library deprecator
instances.  See also rails#46000 and its follow-ups.
  • Loading branch information
jonathanhefner committed Oct 24, 2022
1 parent 8bdea35 commit 09e6442
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 23 deletions.
10 changes: 9 additions & 1 deletion activesupport/lib/active_support/testing/deprecation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@
module ActiveSupport
module Testing
module Deprecation
##
# :call-seq:
# assert_deprecated(&block)
# assert_deprecated(match, &block)
# assert_deprecated(deprecator, &block)
# assert_deprecated(match, deprecator, &block)
#
# Asserts that a matching deprecation warning was emitted by the given deprecator during the execution of the yielded block.
#
# assert_deprecated(/foo/, CustomDeprecator) do
Expand All @@ -19,7 +26,7 @@ module Deprecation
#
# If the +match+ is omitted (or explicitly +nil+), any deprecation warning will match.
#
# assert_deprecated(nil, CustomDeprecator) do
# assert_deprecated(CustomDeprecator) do
# CustomDeprecator.warn "foo should no longer be used"
# end
#
Expand All @@ -29,6 +36,7 @@ module Deprecation
# ActiveSupport::Deprecation.warn "foo should no longer be used"
# end
def assert_deprecated(match = nil, deprecator = nil, &block)
match, deprecator = nil, match if match.is_a?(ActiveSupport::Deprecation)
result, warnings = collect_deprecations(deprecator, &block)
assert !warnings.empty?, "Expected a deprecation warning within the block but received none"
if match
Expand Down
2 changes: 1 addition & 1 deletion activesupport/test/deprecation/deprecators_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,6 @@ def assert_silencing(deprecator)
end

def assert_not_silencing(deprecator)
assert_deprecated(/./, deprecator) { deprecator.warn }
assert_deprecated(deprecator) { deprecator.warn }
end
end
48 changes: 27 additions & 21 deletions activesupport/test/deprecation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ def setup
assert_deprecated(/fubar/, @deprecator) do
@deprecator.warn("using fubar is deprecated")
end

assert_deprecated(@deprecator) do
@deprecator.warn("whatever")
end
end

test "assert_not_deprecated" do
Expand Down Expand Up @@ -303,7 +307,7 @@ def setup

test "assert_deprecated raises when no deprecation warning" do
assert_raises(Minitest::Assertion) do
assert_deprecated(/./, @deprecator) { 1 + 1 }
assert_deprecated(@deprecator) { 1 + 1 }
end
end

Expand Down Expand Up @@ -347,7 +351,7 @@ def setup
assert_not_deprecated(@deprecator) { @deprecator.warn }
end

assert_deprecated(/./, @deprecator) { @deprecator.warn }
assert_deprecated(@deprecator) { @deprecator.warn }

@deprecator.silenced = true
assert @deprecator.silenced
Expand All @@ -364,21 +368,21 @@ def setup
@deprecator.silence { raise }
end

assert_deprecated(/./, @deprecator) { @deprecator.warn }
assert_deprecated(@deprecator) { @deprecator.warn }
end

test "silence only affects the current thread" do
@deprecator.silence do
assert_not_deprecated(@deprecator) { @deprecator.warn }

Thread.new do
assert_deprecated(/./, @deprecator) { @deprecator.warn }
assert_deprecated(@deprecator) { @deprecator.warn }

@deprecator.silence do
assert_not_deprecated(@deprecator) { @deprecator.warn }
end

assert_deprecated(/./, @deprecator) { @deprecator.warn }
assert_deprecated(@deprecator) { @deprecator.warn }
end.join

assert_not_deprecated(@deprecator) { @deprecator.warn }
Expand All @@ -389,8 +393,8 @@ def setup
klass = Class.new(Deprecatee)
klass.deprecate :fubar, :fubar=, deprecator: @deprecator

assert_deprecated(/./, @deprecator) { klass.new.fubar }
assert_deprecated(/./, @deprecator) { klass.new.fubar = :foo }
assert_deprecated(@deprecator) { klass.new.fubar }
assert_deprecated(@deprecator) { klass.new.fubar = :foo }
end

test "Module::deprecate with alternative method" do
Expand Down Expand Up @@ -583,10 +587,10 @@ def method

test "disallowed_warnings with the default warning message" do
@deprecator.disallowed_warnings = :all
assert_disallowed(/./, @deprecator) { @deprecator.warn }
assert_disallowed(@deprecator) { @deprecator.warn }

@deprecator.disallowed_warnings = ["fubar"]
assert_deprecated(/./, @deprecator) { @deprecator.warn }
assert_deprecated(@deprecator) { @deprecator.warn }
end

test "disallowed_behavior callbacks" do
Expand All @@ -600,10 +604,10 @@ def method
test "allow" do
@deprecator.disallowed_warnings = :all

assert_disallowed(/./, @deprecator) { @deprecator.warn }
assert_disallowed(@deprecator) { @deprecator.warn }

@deprecator.allow do
assert_deprecated(/./, @deprecator) { @deprecator.warn }
assert_deprecated(@deprecator) { @deprecator.warn }
end
end

Expand Down Expand Up @@ -641,29 +645,29 @@ def method
@deprecator.disallowed_warnings = :all

@deprecator.allow do
assert_deprecated(/./, @deprecator) { @deprecator.warn }
assert_deprecated(@deprecator) { @deprecator.warn }
end

assert_disallowed(/./, @deprecator) { @deprecator.warn }
assert_disallowed(@deprecator) { @deprecator.warn }
end

test "allow only affects the current thread" do
@deprecator.disallowed_warnings = :all

@deprecator.allow do
assert_deprecated(/./, @deprecator) { @deprecator.warn }
assert_deprecated(@deprecator) { @deprecator.warn }

Thread.new do
assert_disallowed(/./, @deprecator) { @deprecator.warn }
assert_disallowed(@deprecator) { @deprecator.warn }

@deprecator.allow do
assert_deprecated(/./, @deprecator) { @deprecator.warn }
assert_deprecated(@deprecator) { @deprecator.warn }
end

assert_disallowed(/./, @deprecator) { @deprecator.warn }
assert_disallowed(@deprecator) { @deprecator.warn }
end.join

assert_deprecated(/./, @deprecator) { @deprecator.warn }
assert_deprecated(@deprecator) { @deprecator.warn }
end
end

Expand Down Expand Up @@ -695,11 +699,11 @@ def method
@deprecator.disallowed_warnings = :all

@deprecator.allow(:all) do
assert_deprecated(/./, @deprecator) { @deprecator.warn }
assert_deprecated(@deprecator) { @deprecator.warn }
end

@deprecator.allow(["fubar"]) do
assert_disallowed(/./, @deprecator) { @deprecator.warn }
assert_disallowed(@deprecator) { @deprecator.warn }
end
end

Expand Down Expand Up @@ -736,6 +740,7 @@ def with_rails_logger(logger)

# a la collect_deprecations
def collect_disallowed(deprecator)
deprecator ||= ActiveSupport::Deprecation
original_disallowed_behavior = deprecator.disallowed_behavior
disallowed = []
deprecator.disallowed_behavior = proc { |message| disallowed << message }
Expand All @@ -746,7 +751,8 @@ def collect_disallowed(deprecator)
end

# a la assert_deprecated
def assert_disallowed(match = nil, deprecator = ActiveSupport::Deprecation, &block)
def assert_disallowed(match = nil, deprecator = nil, &block)
match, deprecator = nil, match if match.is_a?(ActiveSupport::Deprecation)
result, disallowed = collect_disallowed(deprecator, &block)
assert_not_empty disallowed, "Expected a disallowed deprecation within the block but received none"
if match
Expand Down

0 comments on commit 09e6442

Please sign in to comment.