-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Can not change ActiveJob::Base.queue_adapter in Rails 6 system spec #2232
Comments
My attempt to make a reproduction in straight Rails does not reproduce, it does appear to be a problem specific to This following Rails system test passes in Rails 6.0.2, demonstrating it is possible to change require "application_system_test_case"
class JobsSystemsTest < ApplicationSystemTestCase
class MyJob < ApplicationJob
queue_as :default
def perform(*args)
$my_job_recieved_args << args
end
end
setup do
ActiveJob::Base.queue_adapter = :test
$my_job_recieved_args ||= []
end
test "setting :inline adapter" do
ActiveJob::Base.queue_adapter = :inline
MyJob.perform_later("one")
assert_includes $my_job_recieved_args, ["one"]
assert_instance_of ActiveJob::QueueAdapters::InlineAdapter, ActiveJob::Base.queue_adapter
end
end |
It's appears to have something to do with ActiveJob::TestHelper. I can't figure out what would have changed related to that in Rails 6. I can see how some of what's in there seems problematic, but it seemed problematic in Rails 5.2 also. There is some discussion of related in rails/rails#37270 , but I don't fully understand the discussion there; there was apparently some change in Rails 6.0.2 intended to fix somewhat similar problems, but it does not fix above problem, which does reproduce in Rails 6.0.2 as well. I can't quite figure out where ActiveJob::TestHelper gets included into the class in various versions. But to confirm it does, I check I can't figure out what would have changed to make behavior different with rspec and Rails 6 as above. I've also been trying to figure out a workaround involving explicitly calling ActiveJob::TestHelper's |
We don't tamper with this setting so its extremely odd if its something we're doing, it's more likely an interaction with ordering either of helpers or module inclusion et al, if you can find how they fixed it for minitest for 6.0.2 its likely that change can be applied here to help rspec-rails users. |
Thanks for reply. I can't really figure out what's going on at all. I understand it's not something you're doing, but it's still something that breaks with rspec-rails under Rails 6.0, and makes something useful/necessary to do no longer possible. I suspect many people other than me are going to run into this. Part of the confusion is that the way you do a number of things related to jobs and testing in Rails MiniTest seems pretty different than in rspec-rails. Combined with the fact that there is some weird old legacy code involved in ActiveJob::TestHelper.
So, I'm not sure the "it" is the same, I'm not sure they fixed "it" in minitest/rails. I know that rails/rails#37270 is related somehow, but I'm not certain it's the same issue reported. I'm also not sure it's fixed, it is still open in Rails. This is all very confusing. This is the PR mentioned in rails/rails#37270 as a fix... but I can't figure out why it wouldn't just apply to rspec-rails too, there's nothing obvious to me in there to port to rspec-rails. If there was anyone else who had time to take a look, especially if they have some experience with this code, it would be very helpful. I'm pretty lost/blocked on it. |
OK wait. That rails change makes Rails Would that be a corresponding change needed in rspec-rails SystemExampleGroup? It looks like it's still got:
And
Does one of or both of those need to be changed corresponding to rails/rails#36283 ? Figuring out how Rails test classes map to Rspec classes and setup is new to me, I'm not totally following all of this. |
Okay, actually, just removing the line
That might corerspond to rails/rails#37270 SystemTestCase "Inherit from ActiveSupport::TestCase instead of ActionDispatch::IntegrationTest"? Not sure if that's inconsistent with the I can make a branch run all rspec-rails tests to see if any break with that. I could use at least a sanity check from someone more familiar with this code and with rspec-rails SystemExampleGroup specifically. Looks like that's maybe @penelopezone and @JonRowe I'm kind of just twiddling bits here, I don't totally follow the principles behind rspec-rails adaptation of rails test classes. |
OK, the specific rails commit that was a backport in rails to 6-0-stable branch, that probably needs to be done equivalently in rspec-rails is rails/rails@ea77dbf It looks like maybe if IntegrationTest::Behavior is removed, some custom url_options stuff might need to e done? I am trying to prepare an rspec-rails PR, but not familiar with what's going on, having trouble figuring out what's going on in rspec-rails own tests, like can't figure out how/if these specs ever get run, and I'm not generally familiar with cucumber. I continue to work on it, but definitely appreciate any tips or ideas, for what's going on in general. |
Thanks for the detailed report @jrochkind. I will be happy to help you if I can in your PR. |
Closed by #2242 |
For anybody looking to fix their tests, also this can be used in all test types:
|
Rails 6.0.2 (or 6.0.1), rspec-rails 4.0.0.beta3, rspec-core 3.9.0.
I am used to being able to set
ActiveJob::Base.queue_adapter
to:inline
in some tests and:test
in others -- depending on nature of test, one or the other may be more convenient.In particular in
system
tests (which I understand are now recommended by rspec-rails over the older rspec-specificfeature
tests), it is often convenient to use:inline
, to get actual behavior.However, under Rails 6, I am unable to set
ActiveJob::Base.queue_adapter
in asystem
test, it remains set to the RailsTestAdapter
.This spec would be expected to pass, but fails:
Above fails. However, it passes if you change to
type: :model
ortype: :job
(or probably any type but:system
).It is a regression in the sense that this identical test will also pass under Rails 5.2.3 , rspec-rails 5.0.0.beta3 (or rspec-rails 4.x), rspec-core 3.9.0.
It is confusing how rspec-rails relates to ordinary rails here. It's possible this is a bug/change in Rails itself, but i haven't yet figured out how to reproduce it for a report to Rails in a way that makes sense.
Either way, I believe this action (setting ActiveJob::Base.queue_adapter on a per-test basis, specifically in
system
tests) is something rspec users will want to do, and were able to do in Rails 5.x, but no seem to be in Rails 6.x. I believe it is a problem.Some related context may be provided in: rails/rails#37270 Although the original reporter there says their problem was resolved by Rails 6.0.2 -- however, the problem being reported here still reproduces in Rails 6.0.2.
The text was updated successfully, but these errors were encountered: