Skip to content

Commit b0b481a

Browse files
Fix teardown callbacks (rails#50915)
* Switch ActiveSupport::TestCase teardown and setup callbacks to run in setup and teardown minitest lifecycle hooks. Minitest provides `setup` and `teardown` lifecycle hooks to run code in. In general it is best practice to when defining your own test case class, to use `Minitest::TestCase.setup` and `Minitest::TestCase.teardown` instead of `before_setup` and `after_teardown`. Per Minitest's Documentation on Lifecycle Hooks: https://docs.ruby-lang.org/en/2.1.0/MiniTest/Unit/LifecycleHooks.html > before_setup() > Runs before every test, before setup. This hook is meant for libraries to extend minitest. It is not meant to be used by test developers. > after_teardown() > Runs after every test, after teardown. This hook is meant for libraries to extend minitest. It is not meant to be used by test developers. Since the `setup` and `teardown` ActiveSupport::TestCase callbacks are in essence user code, it makes sense to run during their corresponding Minitest Lifecycle hooks. * Ensure test fixutres are torndown on errors in superclass after_teardown code. By not adding wrapping the `teardown_fixtures` code, its possible that super raises an error and prevents the existing database transaction from rolling back. `super` in general should only be calling `Minitest::Testcase.after_teardown` however, if another library were to override `Minitest::Testcase.after_teardown`, like the popular gem [rspec-mocks](https://github.com/rspec/rspec-mocks/blob/main/lib/rspec/mocks/minitest_integration.rb#L23) does, it causes all subsequent tests to retain any changes that were made to the database in the original test that errors. * Remove unnecessary setup and teardown methods in tests * update activesupport Changelog * Fix linter issues in CHANGELOG * fix tests with improper setup and teardown method definitions * Fix final CHANGELOG lint * Revert "Fix final CHANGELOG lint" This reverts commit f30682e. * Revert "fix tests with improper setup and teardown method definitions" This reverts commit 1d5b88c. * Revert "Fix linter issues in CHANGELOG" This reverts commit 60e89bd. * Revert "update activesupport Changelog" This reverts commit 0f19bc3. * Revert "Remove unnecessary setup and teardown methods in tests" This reverts commit e5673f1. * Revert "Switch ActiveSupport::TestCase teardown and setup callbacks to run in setup and teardown minitest lifecycle hooks." This reverts commit d08d92d. * Rescue Minitest::Assertion errors in ActiveSupport::TestCase.teardown callback code to ensure all other after_teardown methods are called. * Fix name of test class * remove unused MyError class * Fix module to not be in global namespace Co-authored-by: Rafael Mendonça França <[email protected]>
1 parent 316338a commit b0b481a

File tree

3 files changed

+36
-0
lines changed

3 files changed

+36
-0
lines changed

activerecord/lib/active_record/test_fixtures.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ def before_setup # :nodoc:
1313

1414
def after_teardown # :nodoc:
1515
super
16+
ensure
1617
teardown_fixtures
1718
end
1819

activesupport/lib/active_support/testing/setup_and_teardown.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ def after_teardown # :nodoc:
4646
run_callbacks :teardown
4747
rescue => e
4848
self.failures << Minitest::UnexpectedError.new(e)
49+
rescue Minitest::Assertion => e
50+
self.failures << e
4951
end
5052

5153
super
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# frozen_string_literal: true
2+
3+
require_relative "../abstract_unit"
4+
5+
class AfterTeardownAssertionTest < ActiveSupport::TestCase
6+
module OtherAfterTeardown
7+
def after_teardown
8+
super
9+
10+
@witness = true
11+
end
12+
end
13+
include AfterTeardownAssertionTest::OtherAfterTeardown
14+
15+
attr_writer :witness
16+
17+
teardown do
18+
flunk "Test raises a Minitest::Assertion error, all after_teardown should still get called"
19+
end
20+
21+
def after_teardown
22+
assert_changes -> { failures.count }, from: 0, to: 1 do
23+
super
24+
end
25+
26+
assert_equal true, @witness
27+
failures.clear
28+
end
29+
30+
def test_teardown_raise_but_all_after_teardown_method_are_called
31+
assert true
32+
end
33+
end

0 commit comments

Comments
 (0)