RSpec leaks ActiveSupport::ExecutionContext between specs.

[BobbyMcWho]

What Ruby, Rails and RSpec versions are you using?

Ruby version: 2.7 Rails version: 7.0.4 RSpec version: ~> 6.0

Observed behaviour

When using the Rails 7+ built in error reporter Rails.error, any context set during a spec leaks to the next spec.

I've resolved this by

config.before(:each)
  ActiveSupport::ExecutionContext.clear
end

While this works for me, it feels like a tripping point for folks using rspec-rails on rails >= 7.0

Expected behaviour

ExecutionContext should be cleared between each spec, so that context does not leak between specs.

Can you provide an example app?

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'
  gem 'rails', '7.0.4'
  gem 'rspec-rails', '~> 6.0.0'
end

require 'rails'
require 'action_controller/railtie'

class MyApplication < Rails::Application; end

class PagesController < ActionController::Base
  include Rails.application.routes.url_helpers

  def index
    render inline: "SOME HTML"
  end
end

MyApplication.routes.draw do
  get '/pages', to: 'pages#index'
end

require 'rspec/rails'
require "rspec/autorun"

def app
  MyApplication
end

RSpec.describe "First non-controller test" do
  it "does not leak the controller context" do
    expect(ActiveSupport::ExecutionContext.to_h).to eq({})
  end
end

RSpec.describe PagesController, type: :controller do
  it "runs a controller test" do
    get :index
    expect(response.body).to eq("SOME HTML")
    expect(ActiveSupport::ExecutionContext.to_h).to eq({ controller: controller })
  end
end

# fails
RSpec.describe "Second non-controller test" do
  it "does not leak the controller context" do
    expect(ActiveSupport::ExecutionContext.to_h).to eq({})
  end
end

RSpec.describe "A context test" do
  before do
    ActiveSupport::ExecutionContext.clear
    Rails.error.set_context(foo: "bar")
  end

  it "sets the context" do
    expect(ActiveSupport::ExecutionContext.to_h).to eq({foo: "bar"})
  end
end

# fails
RSpec.describe "A second context test" do
  it "does not leak the context" do
    expect(ActiveSupport::ExecutionContext.to_h).to eq({})
  end
end

[pirj]

A quick solution would be to include ActiveSupport::ExecutionContext::TestHelper to RSpec::Rails::RailsExampleGroup.

What is interesting is that a new (can't find it in Rails 6.1 docs) option, executor_around_test_case, which defaults to true since 7.0, basing on which, ActiveSupport::Executor::TestHelper would be included into ActiveSupport::TestCase instead.

Would you be interested in sending a PR, @BobbyMcWho ?

[javierjulio]

@pirj I've been reviewing this for awhile now since I was trying to understand if this would affect our Rails 7 upgrade and to see about contributing a fix/update. But I had been going around in circles trying to understand why this isn't supported and resetting CurrentAttributes is since they are treated the same. I recall some time ago coming across #2503 and with this comment https://github.com/rspec/rspec-rails/issues/2503#issuecomment-851054308 it details why some of the tests in this issue description are failing: they aren't specifying a type, e.g. type: :model. I was going to contribute a change but wonder now if this should be closed out instead, unless Rspec wants to support this and other Rails based helpers outside Rails-ish tests? Perhaps there is a special case here with the new Rails.error feature though.

[bensheldon]

I believe that the best option here is reproducing the behavior of ActiveSupport::Executor::TestHelper / config.active_support.executor_around_test_case and wrapping every example by default with Rails.application.executor.perform {}. It's not just ExecutionContext that isn't cleared, but the absence of a wrapping Executor can also alter Active Record behavior and other Rails behavior that expect to always be wrapped within an Executor.

[JonRowe]

Rails.application.executor.perform {} is not actually enough from testing but including the module if it exists works

[JonRowe]

If someone wants to take up the executor thing you can see it currently blows up various job things for us in #2712, given I don't really know what it affects and most internal rails things will have their own I'm inclined to leave it for now

[bensheldon]

@JonRowe I can take on the Executor wrap, thanks for linking to #2712. I'll make an issue to track it.

I don't really know what it affects and most internal rails things will have their own I'm inclined to leave it for now

In Active Record, it will allow for the query cache to be enabled, it will also appropriately checkout and check-in database connections (and retry/recover from connection errors).

Also, the executor will only be present for integration-like tests because the only places Rails will inserts executors is via a Rack Middleware, or when deserializing a job via perform_later.

[bensheldon]

Created #2713

[JonRowe]

Fix technically released in 6.0.4 but please use 6.1.0 for Rails 7.1 support.