after hook for system tests has no access to the page

[arielj]

What Ruby, Rails and RSpec versions are you using?

Ruby version: 2.6.5
Rails version: 6.1.4.1
RSpec-core: 3.10.1
RSpec-rails: 5.0.2

Observed behaviour

When trying to get the JavaScript code coverage data from the window object on an after hook (shown here https://jtway.co/collecting-javascript-code-coverage-with-capybara-in-ruby-on-rails-application-d0cb83a86a90), when the hook is executed, the page is blank.

The same code using MiniTest works fine (the before_teardown runs before the test is done so the page is at the last state) and it seems like at some point this worked fine on RSpec too since that blogpost describes the process and I've seen the same in other places.

It seems like rspec's after is equivalent to the after_teardown instead of the before_teardown hook from minitest?

Expected behaviour

I would expect the hook to have access to the page as it was at the end of the execution of the example to be able to get the code coverage information from the window object.

Can you provide an example app?

https://github.com/fastruby/js-coverage-sample-app this sample app has both MiniTest and RSpec configured with some code related to getting the JS test coverage.

The only way I'm able to get the JS coverage data is calling the dump_js_coverage method at the end of the test (in spec/system/index_spec.rb). If I try to use an after hook in spec/rails_helper.rb#65, running page.evaluate_script("window.__coverage__") returns nil instead of the coverage data.

You can run COVERAGE=true rails spec to try this.

Same function is being used as a before_teardown hook for MiniTest at test/application_system_test_case.rb.

You can run COVERAGE=true rails test:all to try this.

[arielj]

After some debugging, I found a workaround. I added this to my rails_spec.rb file so it sets the before_teardown function of the system specs so when it executes the original_before_teardown here

rspec-rails/lib/rspec/rails/example/system_example_group.rb

Line 112 in fe95eac

original_before_teardown.bind(self).call

, my code runs in the correct order

module RSpec::Rails::SystemExampleGroup
  def before_teardown
    dump_js_coverage
  end
end

[pirj]

after is equivalent to the after_teardown instead of the before_teardown hook

By looking at the adapter, and that around executes after any after hooks, I would say that if you put an

after { dump_js_coverage }

in the example itself, it would execute after before_teardown and before after_teardown.

teardown itself is run as an after hook, too.

It boils down to the order of hook definitions on what runs first, your after hook with dump_js_coverage, or teardown.
Since after is prepended by default (as that it's aliased to prepend_after tells), it is a bit weird that your config.after runs after the one defined by teardown.
My guess is that the order is different with config-level after.

One way around this I can think of is to use a shared context:

RSpec.shared_context "dump JS coverage" do
  after { dump_js_coverage }
end

RSpec.configure do |config|
  config.include_context "dump JS coverage", type: :system
end

Monkey-patching RSpec::Rails::SystemExampleGroup is fine, but I believe you can get the same result with:

# spec/rails_helper.rb
module DumpJSCoverage
  def before_teardown
    dump_js_coverage
  ensure
    super
  end
end

RSpec.configure do |config|
  config.include DumpJSCoverage, type: :system
  ...

Unfortunately, I failed to quickly validate any of those theories due to webdrivers failing to find Chrome binary.

Do you think there's anything actionable left?

[arielj]

Thank you for the quick reply and great suggestions!

The shared context worked (page still have access to the window.__coverage__ object and the content)

Adding after { dump_js_coverage } inside the RSpec.describe block worked (same result)

Including the DumpJSCoverage module for system tests didn't work (page is blank and window.__coverage__ is undefined)

I like the shared content workaround/solution, the monkey patch felt too hacky.

So I'm wondering, is the current behavior the expected one? or is it still a bug? maybe I was expecting the wrong things and this can be closed if that's the case, I understand that at some point in time the after hook should have worked since the blogpost I used for reference is from 2020

[pirj]

It is indeed confusing that config.after and example group level after run on the different sides of teardown.
It would be ideal if teardown was called from a hook added with append_after. This way, any other after hooks will be run before teardown. It would still be possible to run post-teardown hooks by defining them with around.
Or maybe with append_after, but that would probably repeat this situation where definition order matters.
This will provide more flexibility.

This would be a breaking change, though, as now users' config-level after hooks will be run before teardown, not after as previously.

I suggest adding documentation for this case when you need to run something before teardown and after example, and you define this on the config level, not the spec itself. features/system_specs/system_specs.feature seems to be a reasonable place to add such a doc.
Let's see if @JonRowe supports the currently suggested approach with config.include_context, or has a better idea. And a pull request is welcome!

[JonRowe]

We document the order of hooks, but I don't think we document where in that order we integrate Rails helpers which makes this confusing, I do sort of think this is a bug but as we use RSpec to integrate Rails here and RSpec Core has no distinction that matches before / after teardown its sort of luck of the draw, we could possibly use prepend_after for Rails integrations which would sort of emulate these options.

[timdiggins]

The narrow definition of this issues given in the title (after hooks have no access to page) should now be resolved in #2596.

(However the issues around documentation are not resolved. The after_teardown hooks are now being run as late as possible (in an around hook). This also means that a user who wants to run something after those after_teardown hooks may still find that tricky.)