From ef12ccfd8bc42d88611dea1190988214836b951c Mon Sep 17 00:00:00 2001 From: Richard Macklin Date: Sat, 20 Apr 2019 19:09:50 -0700 Subject: Make system tests take failed screenshots in `before_teardown` hook Previously we were calling the `take_failed_screenshot` method in an `after_teardown` hook. However, this means that other teardown hooks have to be executed before we take the screenshot. Since there can be dynamic updates to the page after the assertion fails and before we take a screenshot, it seems desirable to minimize that gap as much as possible. Taking the screenshot in a `before_teardown` rather than an `after_teardown` helps with that, and has a side benefit of allowing us to remove the nested `ensure` commented on here: https://github.com/rails/rails/pull/34411#discussion_r232819478 --- .../system_testing/test_helpers/setup_and_teardown.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'actionpack/lib/action_dispatch/system_testing/test_helpers') diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb index 600e9c733b..7080dbe022 100644 --- a/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb +++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/setup_and_teardown.rb @@ -16,12 +16,14 @@ module ActionDispatch super end + def before_teardown + take_failed_screenshot + ensure + super + end + def after_teardown - begin - take_failed_screenshot - ensure - Capybara.reset_sessions! - end + Capybara.reset_sessions! ensure super end -- cgit v1.2.3