Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jobs scheduled to run in the future never run in ActionDispatch::IntegrationTests #846

Open
pgvsalamander opened this issue Feb 14, 2023 · 13 comments

Comments

@pgvsalamander
Copy link
Contributor

For some reason there is different behavior for tests that subclass ActiveSupport::TestCase vs ActionDispatch::IntegrationTest. Any jobs that are run using perform_later are never executed for integration tests (perform_now is unaffected). I admit that running a background job directly in an integration test doesn't make much sense, that was just the most terse way to reproduce the issue.

Platform/versions

Ubuntu 22 via WSL 2 on Windows 10
Ruby 2.6.5
Rails 6.1.5.1
GoodJob 3.0.0/3.12.1 (occurs on both)

Steps to Reproduce:

/config/application.rb
class Application < Rails::Application
  ...
  config.active_job.queue_adapter = :good_job
  config.good_job.execution_mode = :inline
  ...
end

/app/jobs/demo_job.rb
class DemoJob < ApplicationJob
	def perform
		ap "This is the demo job"
		raise_an_exception_to_make_noise
	end
end

/test/controllers/action_dispatch_integration_test.rb
class ActionDispatchIntegrationTest < ActionDispatch::IntegrationTest
	test "background job issue demo" do
		DemoJob.perform_later

		travel_to(1.year.from_now) do
			GoodJob.perform_inline
		end

		ap "All jobs should have run at this point"
	end
end

/test/models/normal_unit_test.rb
class NormalUnitTest < ActiveSupport::TestCase
	test "background job issue demo" do
		DemoJob.perform_later

		travel_to(1.year.from_now) do
			GoodJob.perform_inline
		end

		ap "All jobs should have run at this point"
	end
end

For the code above, the NormalUnitTest test will fail because of the undefined method, while the ActionDispatchIntegrationTest test will finish without ever executing the job.

Observations

I think the root of this issue comes down to different queue adapters being set in the different tests, the normal tests use the GoodJob adapter while the integration tests use the stock ActiveJob adapter. When checking ActiveJob::Base.queue_adapter in each test:

  • ActiveSupport::TestCase has an <ActiveJob::QueueAdapters::GoodJobAdapter> instance
  • ActionDispatch::IntegrationTest has an <ActiveJob::QueueAdapters::TestAdapter> instance

When I inspect the queue adapter for the integration test it shows that all the jobs are queued up, but GoodJob can't find them when it goes to execute all pending jobs.

Related

#435 - It looks like a similar (same?) issue came up before, but it was closed without the underlying bug being fixed

rails/rails#37270 - linked in #435, this may actually be a rails issue instead of a GJ issue? I'm not familiar enough with how the queue adapter gets setup to say if this is Rails clobbering GoodJob, or GoodJob not setting itself up properly in certain conditions. I can look into it further if this is accepted as an issue but I wanted to see if this was already a known behavior.

@bensheldon
Copy link
Owner

bensheldon commented Feb 16, 2023

It's weird that the behavior is different between ActiveSupport::TestCase and ActionDispatch::IntegrationTest. I think this is an issue caused by rails/rails#37270

Could you try doing something like this in the ActionDispatch::IntegrationTest?

def setup
  (ActiveJob::Base.descendants + [ActiveJob::Base]).each(&:disable_test_adapter)
end

I think that would cause Rails to respect the queue_adapter assignment.

Edit: and to clarify, if the queue adapter is <ActiveJob::QueueAdapters::TestAdapter>, then the jobs won't be queued into GoodJob and thus GoodJob won't have the jobs to execute.

@pgvsalamander
Copy link
Contributor Author

Sorry for the delayed reply, I could've sworn I replied earlier but I must've forgot to hit post.

We're using this line in our test setup to workaround the issue - queue_adapter_changed_jobs.each(&:disable_test_adapter), I think our two snippets are pretty much equivalent.

Seeing as this seems to be an issue with rails, would you like this issue kept open to track the other or would you like me to close it?

@amo13
Copy link

amo13 commented Jun 25, 2023

Unfortunately, both of your workaround suggestions make the issue worse for me:

  • enqueuing jobs in a ActiveSupport::TestCase fails
  • and in my ActionDispatch::IntegrationTest I get the following error:
Error:
AccountControllerTest#test_reset_password_with_email:
NoMethodError: undefined method `perform_enqueued_jobs' for #<ActiveJob::QueueAdapters::GoodJobAdapter:0x00007f6f58e3dc70
@_execution_mode_override=nil,
@capsule=#<GoodJob::Capsule:0x00007f6f5ab61630
@configuration=#<GoodJob::Configuration:0x00007f6f59afdcd0
@options={},
@autostart=true,
@running=false,
@mutex=#<Thread::Mutex:0x00007f6f59afc628>>>
    test/controllers/user_controllers_test.rb:241:in `block in <class:AccountControllerTest>'

@bensheldon
Copy link
Owner

Can you share the code at test/controllers/user_controllers_test.rb:241? The exception says perform_enqueued_jobs is being called on the queue adapter, which isn't correct (it should be called on the Test Case).

Would you be able to share your codebase with me? I think it will be tough to debug otherwise.

@amo13
Copy link

amo13 commented Jun 25, 2023

One of the tests that trigger this error is this one:

class AccountControllerTest < ActionDispatch::IntegrationTest
  include Devise::Test::IntegrationHelpers

  def setup
    (ActiveJob::Base.descendants + [ActiveJob::Base]).each(&:disable_test_adapter)

    @new_user = {
      email: "[email protected]",
      subdomain: "testuser",
      password: "testpassword",
      password_confirmation: "testpassword"
    }
  end

  test "create user and send confirmation and reminder email" do
    assert_emails 1 do
      assert_difference("User.count") do
        post user_registration_url, params: {
          user: @new_user
        }
      end
    end

    assert_redirected_to account_overview_path

    email = ActionMailer::Base.deliveries.last
    assert_equal [@new_user[:email]], email.to

    assert_emails 1 do
      assert_enqueued_with(
        job: SendAccountConfirmationReminderJob,
        args: [{user_id: User.find_by(email: @new_user[:email]).id}],
        queue: "test_accounts",
        priority: 5
      )

      travel(Devise.confirm_within - 3.days) do
        GoodJob.perform_inline
      end
    end
  end
 
end

With (ActiveJob::Base.descendants + [ActiveJob::Base]).each(&:disable_test_adapter) I get the above error. Without I just get the test failure 1 emails expected, but 0 were sent. for the second email assertion because the job is not executed.

(I am very thankful for your help with this! I'd prefer not to share the full codebase though. But I'm happy to share other specific parts that can help debug.)

@bensheldon
Copy link
Owner

bensheldon commented Jun 25, 2023

Thanks! Can you tell me which line in the above code triggers NoMethodError: undefined method 'perform_enqueued_jobs' for #<ActiveJob::QueueAdapters::GoodJobAdapter

@amo13
Copy link

amo13 commented Jun 25, 2023

Yes, I wrapped everything in a begin-rescue-end and asked for the backtrace on the raised exception. And here it is:

/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/activejob-7.0.5/lib/active_job/test_helper.rb:603:in `perform_enqueued_jobs'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/actionmailer-7.0.5/lib/action_mailer/test_helper.rb:37:in `assert_emails'
/home/amo/code/rails/some-project/test/controllers/user_controllers_test.rb:131:in `block in <class:AccountControllerTest>'  ### This is the first "assert_emails 1 do" in the abobe code
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest/test.rb:102:in `block (3 levels) in run'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest/test.rb:199:in `capture_exceptions'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest/test.rb:97:in `block (2 levels) in run'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:296:in `time_it'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest/test.rb:96:in `block in run'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:391:in `on_signal'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest/test.rb:247:in `with_info_handler'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest/test.rb:95:in `run'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/activesupport-7.0.5/lib/active_support/executor/test_helper.rb:5:in `block in run'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/activesupport-7.0.5/lib/active_support/execution_wrapper.rb:105:in `perform'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/activesupport-7.0.5/lib/active_support/executor/test_helper.rb:5:in `run'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:1051:in `run_one_method'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:365:in `run_one_method'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:352:in `block (2 levels) in run'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:351:in `each'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:351:in `block in run'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:391:in `on_signal'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:378:in `with_info_handler'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:350:in `run'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/railties-7.0.5/lib/rails/test_unit/line_filtering.rb:10:in `run'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:182:in `block in __run'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:182:in `map'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:182:in `__run'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:159:in `run'
/home/amo/.rvm/gems/ruby-3.2.2@rails7/gems/minitest-5.18.0/lib/minitest.rb:83:in `block in autorun'

Thank you for investigating.

The last caller appears to be the test-helper.rb of active_job at line 603.

@bensheldon
Copy link
Owner

@amo13 ohh, the issue here is that you're using test helpers that rely upon the Active Job TestAdapter e.g. assert_emails calls deliver_enqueued_emails calls perform_enqueued_jobs.

If you're doing "integration testing" using a real queue adapter like GoodJob, any test helpers relying on the Active Job TestAdapter won't work.

I think I'd be ok if the GoodJob Adapter quacked like a TestAdapter and implemented enqueued_jobs and performed_jobs (and they simply made database queries and returned the serialized data)... or maybe that would just lead to even more confusion.

@amo13
Copy link

amo13 commented Jun 27, 2023

Thank you so much. I am still very new to rails.
Can you please clarify for me what I need to do to make my tests run? My understanding of the GoodJob readme tests section is that GoodJob can be used in integration tests. Or is an integration test simply the wrong place to do email assertions?

@bensheldon
Copy link
Owner

ah gotcha. The problem you're experiencing is that you're combining two different integration test strategies that aren't compatible.

  • Use the Active Job TestAdapter, and the job / email assertion helpers
  • Use an actual Active Job adapter (GoodJob) and assert directly on outcomes (rather than implementation) you expect.

Using the second strategy, here is how I would test it:

class AccountControllerTest < ActionDispatch::IntegrationTest
  include Devise::Test::IntegrationHelpers

  def setup
    (ActiveJob::Base.descendants + [ActiveJob::Base]).each(&:disable_test_adapter)

    @new_user = {
      email: "[email protected]",
      subdomain: "testuser",
      password: "testpassword",
      password_confirmation: "testpassword"
    }
  end

  test "create user and send confirmation and reminder email" do
    post user_registration_url, params: {
      user: @user_data
    }
    assert_redirected_to account_overview_path

    user = User.last
    assert_equal @new_user.email, user.email

    invite_email = ActionMailer::Base.deliveries.last
    assert_equal [@new_user[:email]], invite_email.to
    assert_equal "Welcome!", invite_email.subject

    travel(Devise.confirm_within - 3.days) do
      GoodJob.perform_inline
    end

    reminder_email = ActionMailer::Base.deliveries.last
    assert_equal [@new_user[:email]], reminder_email.to
    assert_equal "Don't forget to confirm!", reminder_email.subject
  end
end

@amo13
Copy link

amo13 commented Jun 28, 2023

Thank you very much! I think I begin to understand.


I think I'd be ok if the GoodJob Adapter quacked like a TestAdapter and implemented enqueued_jobs and performed_jobs (and they simply made database queries and returned the serialized data)... or maybe that would just lead to even more confusion.

For me, that would be awesome. Making a GoodJob TestAdapter class inheriting from the actual GoodJob adapter and implementing these methods might prevent the confusion. One could then for example declare something like config.active_job.queue_adapter = :good_job_test in environments/test.rb.

@ghiculescu
Copy link

^ I have rails/rails#48585 in progress to fully solve this issue. And rails/rails#48599 as a smaller PR that should resolve the inability to use assert_emails.

@Timmitry
Copy link
Contributor

The PR from @ghiculescu is merged and included in Rails 7.2 - I think this should resolve this issue?

I just stumbled upon changed job behaviour in my tests when upgrading Rails, which made some tests fail, which is how I noticed this 🙂 Thank you for fixing that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Inbox
Development

No branches or pull requests

5 participants