Issues with take_failed_screenshot after PR 36047

[tatethurston]

After updating to Rails 6, Capybara acceptance test failure output changed, and screenshots for failures are now blank (empty white image).

The test summary changes make it difficult to associate failed test cases with the corresponding screenshot.

It looks like both issues stem from https://github.com/rails/rails/pull/36047/files, which moved failed screenshots into a before_teardown hook.

Reverting the changes from https://github.com/rails/rails/pull/36047/files fixes both issues.

Steps to reproduce

Expected behavior

Maintain the same behavior as Rails 5.2:

1. Screenshot path is printed in the test summary
2. Screenshots show the page (not a blank)

Actual behavior

Rails 5.2

F....

Failures:

  1) Some Test
     Failure/Error: expect(page).to have_text("foo")
       expected to find text "foo" in "Hello World\n"

     [Screenshot]: tmp/screenshots/failures_r_spec_example_groups_some_test_943.png


     # ./spec/system/some_test_spec.rb:18:in `block (3 levels) in <main>'

Rails 6.0

[Screenshot]: /Users/tatethurston/panorama/tmp/screenshots/failures_r_spec_example_groups_some_test_652.png
F....

Failures:

  1) Some Test
     Failure/Error: expect(page).to have_text("foo")
       expected to find text "foo" in "Hello World\n"



     # ./spec/system/some_test_spec.rb:18:in `block (3 levels) in <main>'

System configuration

Rails version: 6.0

Ruby version: 2.6.4

[eileencodes]

@rmacklin can you take a look at this?

[rmacklin]

@rmacklin can you take a look at this?

Yes, thank you for bringing it to my attention!

[rmacklin]

Here are my initial findings:

I am unable to reproduce the issue when forcing a system test to fail in an application I just upgraded to Rails 6.0.0. Unfortunately, this repository is closed source so I can't show details.

However, I happen to have an open source repository at https://github.com/rmacklin/rails_parallel_testing_experiments that has a fresh rails 5.2.3 application and a fresh rails 6.0.0.rc1 application. I created two new branches in that repository where I produced a failing system test in each application. Below are the details:

---

Here is a failing CircleCI build for the rails 5 application:
https://circleci.com/gh/rmacklin/rails_parallel_testing_experiments/45#artifacts/containers/0
and here is a link to the failure screenshot from the artifacts:
https://45-185632913-gh.circle-artifacts.com/0/home/circleci/repo/tmp/screenshots/failures_test_visiting_the_index.png

Here is a failing CircleCI build for the rails 6 application:
https://circleci.com/gh/rmacklin/rails_parallel_testing_experiments/46#artifacts/containers/0
and here is a link to the failure screenshot from the aritfacts:
https://46-185632913-gh.circle-artifacts.com/0/home/circleci/repo/tmp/screenshots/failures_test_visiting_the_index.png

As you can see, neither screenshot is an empty white page (in this sample application the only thing that's rendered is an <h1> element on an otherwise empty white page, but you can see this <h1> in the screenshots, so screenshot capturing is working).

Additionally, here is the terminal log from the rails 5 app:

# Running:

Capybara starting Puma...
* Version 3.12.1 , codename: Llamas in Pajamas
* Min threads: 0, max threads: 4
* Listening on tcp://127.0.0.1:33797
..[Screenshot]: tmp/screenshots/failures_test_visiting_the_index.png
F

Failure:
SampleSystemTest#test_visiting_the_index [/home/circleci/repo/test/system/sample_system_test.rb:7]:
expected to find visible css "h1" with text "Home#indexFAIL" but there were no matches. Also found "Home#index", which matched the selector but not all filters. 


bin/rails test test/system/sample_system_test.rb:4

....
[Minitest::CI] Generating test report in JUnit XML format...


Finished in 16.319913s, 0.4289 runs/s, 0.4289 assertions/s.
7 runs, 7 assertions, 1 failures, 0 errors, 0 skips
Exited with code 1

and here is the terminal log from the rails 6 app:

# Running:

Capybara starting Puma...
* Version 3.12.1 , codename: Llamas in Pajamas
* Min threads: 0, max threads: 4
* Listening on tcp://127.0.0.1:34139
[Screenshot]: /home/circleci/repo/tmp/screenshots/failures_test_visiting_the_index.png
F

Failure:
SampleSystemTest#test_visiting_the_index [/home/circleci/repo/test/system/sample_system_test.rb:7]:
expected to find visible css "h1" with text "Home#indexFAIL" but there were no matches. Also found "Home#index", which matched the selector but not all filters. 


rails test test/system/sample_system_test.rb:4

......

Finished in 16.805272s, 0.4165 runs/s, 0.4165 assertions/s.
7 runs, 7 assertions, 1 failures, 0 errors, 0 skips

[Minitest::CI] Generating test report in JUnit XML format...
Exited with code 1

As you can see, in both rails 5 and rails 6, the failure screenshot is printed directly above the failure message.

---

Basically I was unable to reproduce either issue. Ultimately this isn't too surprising because I'm the person who submitted PR #36047 and I didn't hit either issue when I tested those changes.

---

That isn't satisfying though -- what's going on with this issue that has several 👍 reactions?

Let's take another look at the issue description... Oh:

     Failure/Error: expect(page).to have_text("foo")
       expected to find text "foo" in "Hello World\n"

     [Screenshot]: tmp/screenshots/failures_r_spec_example_groups_some_test_943.png


     # ./spec/system/some_test_spec.rb:18:in `block (3 levels) in <main>'

It looks like this test was written for rspec rather than minitest (the default test framework in rails apps). That's probably why I'm unable to reproduce it in fresh/"stock" rails apps.

As an FYI, I've never used rspec's system tests. But I found the "Integrate with ActionDispatch::SystemTest" PR in rspec-rails and it looks like there's quite a bit more going on than just include ActionDispatch::SystemTesting::TestHelpers::SetupAndTeardown:

https://github.com/rspec/rspec-rails/pull/1813/files#diff-779a18a53423de804d5cbdc1edc6552dR19-R29

The code there is reaching for ActionDispatch::SystemTesting::TestHelpers::SetupAndTeardown's after_teardown method and saving a reference to call it later. Since #36047 moved screenshot capturing to the before_teardown method, it seems that code probably needs to be extended with similar special handling of ActionDispatch::SystemTesting::TestHelpers::SetupAndTeardown's before_teardown method.

TLDR

I suspect this is only an issue when using rspec-rails because I'm unable to reproduce it in a fresh rails 6 app (using minitest). If anyone who 👍'd this issue can reproduce using minitest, could you please share a public github repo that demonstrates the issue? Otherwise, I think we may want to move this issue to rspec-rails.

[rmacklin]

Turns out there already was an issue opened about this on the rspec-rails repository when rails 6.0.0.rc1 was out: rspec/rspec-rails#2153 and it was fixed last month in rspec/rspec-rails#2164.

Note, however, that that PR was merged into the 4-0-dev branch (which is the branch that supports rails 6, per rspec/rspec-rails#2153 (comment)), so it is currently not in rspec-rails@master. Looks like you can subscribe to this PR to be notified when it hits master: rspec/rspec-rails#2071.

@eileencodes do you think we can close this issue? The author of the rspec-rails issue reported that the fix resolved their problems with screenshots: rspec/rspec-rails#2153 (comment).

[eileencodes]

Thanks for the followup @rmacklin! Closing here.