diff options
author | Edouard CHIN <edouard.chin@shopify.com> | 2018-04-26 14:38:02 -0400 |
---|---|---|
committer | Edouard CHIN <edouard.chin@shopify.com> | 2018-04-27 01:36:27 -0400 |
commit | 1cf8b6c23120e24707a2606a4b159a56faf38212 (patch) | |
tree | a8fa1073eba0574d2901e3ad5bff2edc69c193e5 /activesupport/test/testing | |
parent | 7d2400ab61c8e3ed95e14d03ba3844e8ba2e36e4 (diff) | |
download | rails-1cf8b6c23120e24707a2606a4b159a56faf38212.tar.gz rails-1cf8b6c23120e24707a2606a4b159a56faf38212.tar.bz2 rails-1cf8b6c23120e24707a2606a4b159a56faf38212.zip |
`SetupAndTeardown` has few caveats that breaks libraries:
- In #32472 I introduced a fix in order for all `after_teardown` method provided by libraries and Rails to run, even if the application's `teardown` method raised an error (That's the default minitest behavior). However this change wasn't enough and doesn't take in consideration the ancestors chain.
If a library's module containing an `after_teardown` method get included after the `SetupAndTeardown` module (one example is the [ActiveRecord::TestFixtures module](https://github.com/rails/rails/blob/7d2400ab61c8e3ed95e14d03ba3844e8ba2e36e4/activerecord/lib/active_record/fixtures.rb#L855-L856), then the ancestors of the test class would look something like
```ruby
class MyTest < ActiveSupport::TestCase
end
puts MyTest.ancestors # [MyTest, ActiveSupport::TestCase, ActiveRecord::TestFixtures, ActiveSupport::Testing::SetupAndTeardown]
```
Any class/module in the ancestors chain that are **before** the `ActiveSupport::Testing::SetupAndTeardown` will behave incorrectly:
- Their `before_setup` method will get called **after** all regular setup method
- Their `after_teardown` method won't even get called in case an exception is raised inside a regular's test `teardown`
A simple reproduction script of the problem here https://gist.github.com/Edouard-chin/70705542a59a8593f619b02e1c0a188c
- One solution to this problem is to have the `AS::SetupAndTeardown` module be the very first in the ancestors chain. By doing that we ensure that no `before_setup` / `after_teardown` get executed prior to running the teardown callbacks
Diffstat (limited to 'activesupport/test/testing')
-rw-r--r-- | activesupport/test/testing/after_teardown_test.rb | 8 |
1 files changed, 5 insertions, 3 deletions
diff --git a/activesupport/test/testing/after_teardown_test.rb b/activesupport/test/testing/after_teardown_test.rb index 68c368909c..961af49479 100644 --- a/activesupport/test/testing/after_teardown_test.rb +++ b/activesupport/test/testing/after_teardown_test.rb @@ -4,13 +4,14 @@ require "abstract_unit" module OtherAfterTeardown def after_teardown + super + @witness = true end end -class AfterTeardownTest < Minitest::Test +class AfterTeardownTest < ActiveSupport::TestCase include OtherAfterTeardown - include ActiveSupport::Testing::SetupAndTeardown attr_writer :witness @@ -21,11 +22,12 @@ class AfterTeardownTest < Minitest::Test end def after_teardown - assert_raises MyError do + assert_changes -> { failures.count }, from: 0, to: 1 do super end assert_equal true, @witness + failures.clear end def test_teardown_raise_but_all_after_teardown_method_are_called |