-
Notifications
You must be signed in to change notification settings - Fork 21.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Use case A very common pattern in Ruby, especially in testing is to save the value of an attribute, set a new value, and then restore the old value in an `ensure` clause. e.g. in unit tests ```ruby def test_something_when_enabled enabled_was, SomeLibrary.enabled = SomeLibrary.enabled, true # test things ensure SomeLibrary.enabled = enabled_was end ``` Or sometime in actual APIs: ```ruby def with_something_enabled enabled_was = @enabled @enabled = true yield ensure @enabled = enabled_was end ``` There is no inherent problem with this pattern, but it can be easy to make a mistake, for instance the unit test example: ```ruby def test_something_when_enabled some_call_that_may_raise enabled_was, SomeLibrary.enabled = SomeLibrary.enabled, true # test things ensure SomeLibrary.enabled = enabled_was end ``` In the above if `some_call_that_may_raise` actually raises, `SomeLibrary.enabled` is set back to `nil` rather than its original value. I've seen this mistake quite frequently. Object#with I think it would be very useful to have a method on Object to implement this pattern in a correct and easy to use way. NB: `public_send` is used because I don't think such method should be usable if the accessors are private. With usage: ```ruby def test_something_when_enabled SomeLibrary.with(enabled: true) do # test things end end ``` ```ruby GC.with(measure_total_time: true, auto_compact: false) do # do something end ``` Lots of tests in Rails's codebase could be simplified, e.g.: - Changing `Thread.report_on_exception`: https://github.com/rails/rails/blob/2d2fdc941e7497ca77f99ce5ad404b6e58f043ef/activerecord/test/cases/connection_pool_test.rb#L583-L595 - Changing a class attribute: https://github.com/rails/rails/blob/2d2fdc941e7497ca77f99ce5ad404b6e58f043ef/activerecord/test/cases/associations/belongs_to_associations_test.rb#L136-L150
- Loading branch information
Showing
4 changed files
with
141 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
# frozen_string_literal: true | ||
|
||
class Object | ||
# Sets and restore the provided attributes around a block. | ||
# | ||
# client.timeout # => 5 | ||
# client.with(timeout: 1) do | ||
# client.timeout # => 1 | ||
# end | ||
# client.timeout # => 5 | ||
# | ||
# This method is a shorthand for the common begin/ensure pattern: | ||
# | ||
# old_value = object.attribute | ||
# begin | ||
# object.attribute = new_value | ||
# # do things | ||
# ensure | ||
# object.attribute = old_value | ||
# end | ||
# | ||
# It can be used on any object as long as both the reader and writer methods | ||
# are public. | ||
def with(**attributes) | ||
old_values = {} | ||
begin | ||
attributes.each do |key, value| | ||
old_values[key] = public_send(key) | ||
public_send("#{key}=", value) | ||
end | ||
yield | ||
ensure | ||
old_values.each do |key, old_value| | ||
public_send("#{key}=", old_value) | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
# frozen_string_literal: true | ||
|
||
require_relative "../../abstract_unit" | ||
require "active_support/core_ext/object/with" | ||
|
||
class BlankTest < ActiveSupport::TestCase | ||
class Record | ||
def initialize | ||
@public_attr = :public | ||
@another_public_attr = :another_public | ||
@mixed_attr = :mixed | ||
@protected_attr = :protected | ||
@private_attr = :private | ||
end | ||
|
||
attr_accessor :public_attr, :another_public_attr | ||
|
||
attr_reader :mixed_attr | ||
|
||
protected | ||
attr_accessor :protected_attr | ||
|
||
private | ||
attr_accessor :private_attr | ||
attr_writer :mixed_attr | ||
end | ||
|
||
setup do | ||
@object = Record.new | ||
end | ||
|
||
test "sets and restore attributes around a block" do | ||
assert_equal :public, @object.public_attr | ||
assert_equal :another_public, @object.another_public_attr | ||
|
||
@object.with(public_attr: :changed, another_public_attr: :changed_too) do | ||
assert_equal :changed, @object.public_attr | ||
assert_equal :changed_too, @object.another_public_attr | ||
end | ||
|
||
assert_equal :public, @object.public_attr | ||
assert_equal :another_public, @object.another_public_attr | ||
end | ||
|
||
test "restore attribute if the block raised" do | ||
assert_equal :public, @object.public_attr | ||
assert_equal :another_public, @object.another_public_attr | ||
|
||
assert_raise RuntimeError do | ||
@object.with(public_attr: :changed, another_public_attr: :changed_too) do | ||
assert_equal :changed, @object.public_attr | ||
assert_equal :changed_too, @object.another_public_attr | ||
raise "Oops" | ||
end | ||
end | ||
|
||
assert_equal :public, @object.public_attr | ||
assert_equal :another_public, @object.another_public_attr | ||
end | ||
|
||
test "restore attributes if one of the setter raised" do | ||
assert_equal :public, @object.public_attr | ||
assert_equal :mixed, @object.mixed_attr | ||
|
||
assert_raise NoMethodError do | ||
@object.with(public_attr: :changed, mixed_attr: :changed_too) do | ||
assert false | ||
end | ||
end | ||
|
||
assert_equal :public, @object.public_attr | ||
assert_equal :mixed, @object.mixed_attr | ||
end | ||
|
||
test "only works with public attributes" do | ||
assert_raises NoMethodError do | ||
@object.with(private_attr: :changed) { } | ||
end | ||
|
||
assert_raises NoMethodError do | ||
@object.with(protected_attr: :changed) { } | ||
end | ||
|
||
assert_equal :mixed, @object.mixed_attr | ||
assert_raises NoMethodError do | ||
@object.with(mixed_attr: :changed) { } | ||
end | ||
assert_equal :mixed, @object.mixed_attr | ||
end | ||
end |