Rails 6 inconsistently overrides ActiveJob queue_adapter setting with TestAdapter

[searls]

Summary

Like many folks, I set this in my config/environments/test.rb file:

config.active_job.queue_adapter = :inline  

The reason I do this is to have easy-to-test, deterministic behavior of any potentially async delayed jobs triggered by the application. Under Rails 5, all of my tests (children of ActiveSupport::TestCase and ActionDispatch::SystemTestCase alike) would respect this queue adapter. However, under Rails 6, any built-in Rails test case that includes ActiveJob::TestHelper will also override this setting for all descendants of ActiveJob::Base (relevant source).

The net effect of this is that my perform_later and deliver_later calls to jobs aren't being fired synchronously (or, in fact, at all) during my system tests, which is leading to test failures upon upgrading to Rails 6.

Steps to reproduce

Here are two tests that demonstrates the problem:

require "test_helper"

class JobTest < ActiveSupport::TestCase
  class SomeJob < ActiveJob::Base
    cattr_accessor :job_ran

    def perform
      @@job_ran = true
    end
  end

  def test_some_job
    assert_equal :inline, Rails.application.config.active_job.queue_adapter
    assert_equal ActiveJob::QueueAdapters::InlineAdapter, SomeJob.queue_adapter.class

    SomeJob.perform_later

    assert_equal true, SomeJob.job_ran
  end
end

class JobSystemTest < ActionDispatch::SystemTestCase
  class OtherJob < ActiveJob::Base
    cattr_accessor :job_ran

    def perform
      @@job_ran = true
    end
  end

  def test_other_job
    assert_equal :inline, Rails.application.config.active_job.queue_adapter
    assert_equal ActiveJob::QueueAdapters::InlineAdapter, OtherJob.queue_adapter.class

    OtherJob.perform_later

    assert_equal true, OtherJob.job_ran
  end
end

Expected behavior

I would expect the behavior to have remained as it did under Rails 5, where the active_job.queue_adapter setting would be respected for all of Rails' test types.

Actual behavior

The first test, which descends from ActiveSupport::TestCase will pass and behave as it did under Rails 5. However, the second test, descending from from ActionDispatch::SystemTestCase will now fail under Rails 6, because OtherJob's queue_adapter will have been reset by ActiveJob::TestHelper to be an instance of TestAdapter

JobSystemTest#test_other_job [/Users/justin/code/testdouble/present/test/lib/job_test.rb:33]:
--- expected
+++ actual
@@ -1 +1 @@
-ActiveJob::QueueAdapters::InlineAdapter
+ActiveJob::QueueAdapters::TestAdapter

System configuration

Rails version: 6.0.0 as well as 6-0-stable, as of today.

Ruby version: ruby 2.6.3p62 (2019-04-16 revision 67580) [x86_64-darwin18]

[searls]

By the way, for anyone looking to just disable this, one might write a setup hook like this one, invoking the disable_test_adapter method also added by the ActiveJob::TestHelper:

  def setup
    (ActiveJob::Base.descendants << ActiveJob::Base).each do |job_class|
      job_class.disable_test_adapter
    end
  ensure
    super
  end

[gmcgibbon]

I'm slightly confused by this, I see the same adapter overriding code on 5.2. How is it that the behaviour of Rails 5 is different?

We could move this code to the assertion, but that might be expensive. I'm curious why you need to include ActiveJob::TestHelper if you aren't planning on using it. If you need both, it could be that the best answer is overriding queue_adapter_for_test, or only including ActiveJob::TestHelper in the test cases where it is necessary.

[searls]

@gmcgibbon the difference is that my app's test code isn't doing anything to include ActiveJob::TestHelper, it's getting invoked anyway, and that's the change I saw when upgrading to Rails 6.

[gmcgibbon]

🤦‍♂ My mistake, sorry about that. Yes, the problem was introduced in #33849. However, this is fixed on master by #36283.

I think we need to backport #36283 + #36741 to 6-0-stable to patch this up.

[gmcgibbon]

Backported in ea77dbf, 1a09b6e, and 109aad7. Let me know if you're still having issues against 6-0-stable and we can reopen. Thanks!

[halo]

TL; DR: Upgrading from Rails 5 to 6 caused this bug. However, I cannot confirm that 6-0-stable or master fixed the problem.

Background

When I enqueue a job via Ajax, I'm using the provider_job_id to set up a websocket listener to provide visual feedback on how this particular job is doing.

For some reasone, the provider_job_id is not properly kept between requests when I use the TestAdapter (not sure why, I think it's simply not persisting it). So I've been using Sidekiq even in tests:

# config/application.rb
ActiveJob::Base.queue_adapter = :sidekiq

But I have Sidekiq run the jobs inline in feature tests (that way, the provider_job_id is reliably kept):

# spec/rails_helper.rb
RSpec.configure do |config|
  config.around sidekiq: true do |example|
    Sidekiq::Testing.inline! { example.run }
  end
end

So, my capybara tests started failing as of Rails 6:

RSpec.describe 'Documents', :js, :sidekiq, type: :system do
  it 'can Preview PDFs' do
    visit document_path create(:document)

    click_on 'PDF Preview'

    p ActiveJob::Base.queue_adapter  # <- ActiveJob::QueueAdapters::TestAdapter
    
    # This test passes if I uncommend the following line:
    # (ActiveJob::Base.descendants << ActiveJob::Base).each(&:disable_test_adapter)

    p ActiveJob::Base.queue_adapter  # <- Becomes Sidekiq only when disabling the test adapter

    click_on 'Download generated PDF'
  end
end

I could not even move the patch to the RSpec config.around block, it works only right here, in the test.

I started noticing in my test logs that, in Rails 6, the job is only enqueued but not run. (And when I wrapped the test in perform_enqueued_jobs do, the job ran, but the provider_job_id was not matching from when the job was enqueued compared to when the websocket channel is set up.)

Thank you @searls for the detailed bug report, can you confirm whether 6-0-stable fixed this for you?

@gmcgibbon Did you try the "Steps to reproduce" from the issue description on 6-0-master? I'll try that.

I'm truly sorry I wasn't there when the Rails 6 release candidates were battle tested 😢 Let me know if I can be of further assistance.

[searls]

Thanks for the suggestion, @halo

I just pulled down 6-0-0-stable and can confirm it does not fix this in my case, either

[gmcgibbon]

🤔 I'm unable to reproduce the failures. Here's a branch on a dummy app that shows the provided tests passing: https://github.com/gmcgibbon/rails-issues/tree/37270. It looks like @halo's test should pass on 6-0-stable too.

As for this still being broken in 6.0, we can't retroactively patch an already released version of Rails. We can however backport and make sure this fix gets included in 6.0.1.

@searls can you provide another failing test case if things still aren't working?

[searls]

Thanks @gmcgibbon for reopening. Unfortunately I'm heading out the door for vacation won't have access to a machine for about a week, including to tell whether the dummy tests in my original comment pass or fail under 6-0-0-stable@HEAD.

[beauraF]

Hi,

In my case, the problem is solved for my SystemTestCase by switching to 6-0-stable.
On the other hand, I still have exactly the same problem with my ActionDispatch::IntegrationTest.

---

@gmcgibbon it can be reproduced on your project by switching ActionDispatch::SystemTestCaseto ActionDispatch::IntegrationTest:

require "test_helper"

class JobTest < ActiveSupport::TestCase
  class SomeJob < ActiveJob::Base
    cattr_accessor :job_ran

    def perform
      @@job_ran = true
    end
  end

  def test_some_job
    assert_equal :inline, Rails.application.config.active_job.queue_adapter
    assert_equal ActiveJob::QueueAdapters::InlineAdapter, SomeJob.queue_adapter.class

    SomeJob.perform_later

    assert_equal true, SomeJob.job_ran
  end
end

class JobSystemTest < ActionDispatch::IntegrationTest
  class OtherJob < ActiveJob::Base
    cattr_accessor :job_ran

    def perform
      @@job_ran = true
    end
  end

  def test_other_job
    assert_equal :inline, Rails.application.config.active_job.queue_adapter
    assert_equal ActiveJob::QueueAdapters::InlineAdapter, OtherJob.queue_adapter.class

    OtherJob.perform_later

    assert_equal true, OtherJob.job_ran
  end
end

➜  rails t test/integration/queue_adapter_test.rb
Running via Spring preloader in process 27035
Run options: --seed 63757

# Running:

F

Failure:
JobSystemTest#test_other_job [/Users/beauraf/src/rails-issues/test/integration/queue_adapter_test.rb:33]:
--- expected
+++ actual
@@ -1 +1 @@
-ActiveJob::QueueAdapters::InlineAdapter
+ActiveJob::QueueAdapters::TestAdapter



rails test test/integration/queue_adapter_test.rb:31

[gmcgibbon]

Thanks @beauraF. That's precisely what #36283 was trying to sidestep for system test cases. I'm thinking we might have to change the behaviour of ActiveJob::TestHelper because it looks like its inclusion in integration tests is intentional. It seems we likely have to delay enabling the test adapter on jobs until just before we assert.