Add Active Job's assertions to ActiveSupport::TestCase

[robin850]

Hello,

Since we should test job queuing/execution inside other components of the application like models and controllers, we should make the custom assertions provided by ActiveJob::TestHelper available directly from ActiveSupport::TestCase.

Thus, we don't need the ActiveJob::TestCase class anymore so we can remove it and avoid relying on it when we generate a job's test.

This has originally been requested here but it turns out that this hasn't been addressed.

Have a nice day.

[seuros]

👍

[seuros]

Wait! That will break if AJ is not part of the bundle. eq(A project without AJ or AM)

[robin850]

Wait! That will break if AJ is not part of the bundle

Yup, this is not ideal and this is why I didn't put it directly under ActiveSupport::TestCase but well, there's no --skip-active-job option so if you don't want to use Active Job you will have to change things manually anyway. What do you think ?

[seuros]

Do we have --skip-action-mailer ? That the only gem that depend on AJ for now. Could we use it ?

[robin850]

Do we have --skip-action-mailer ?

Nope we don't plus we are adding config.action_mailer statements in the environments files so I'm not sure whether there's a real problem here but let's wait for some feedback, what do you think ? :-)

[chancancode]

That will break if AJ is not part of the bundle

This is not a concern, it has been decided that AJ will always be present in Rails apps. In a way, it's kind of like Rails.cache or Rails.logger so that (eventually – when people drop support for 4.2 and below) library authors etc can depend on the same API being present (there's no point turning it "off" – that's basically the default inline adapter).

[chancancode]

Not sure if this is the right place to add this. As mentioned in my comments above, this is not something that we expect people to turn off, so I don't think it makes sense to include it in a template. Seems like we can just do this automatically somewhere inside Active Job?

[robin850]

Ok, so I moved this directly to Active Job and addressed the discussion that we had on the Campfire room regarding the removal of AcitveJob::TestCase. The only problem is that Active Job's integration tests are now failing.

The problem is that ActiveJob::TestHelper overrides the queue_adapter in the before_setup hook while the integration tests are on a per adapter basis. A simple fix could be to override the adapter in the setup hook of the integration test like setup { ActiveJob::Base.queue_adapter = ENV["AJADAPTER"] } but then we are overriding the adapter three different times (in the job manager of the adapter that we are testing against, inside TestHelper and finally bring it back inside the test).

The real question is do we expect people to be able to use another queue adapter than :test when testing ?

If yes, then we would certainly need a kind of AbstractAdapter like there is in Active Record that'd define common methods needed for testing (e.g. enqueued_jobs) so assertions can work for different adapters. The problem with that approach is that we would add enqueued and performed jobs to their respective arrays while it's pretty useless outside the scope of testing.

If no, then there's no big deal, this won't work anyway outside of the framework but apart from the aforementioned solution with setup, I don't know how we could handle it in our internal test suite. 😕

Feel free to ask questions if something isn't clear in my explanations !

[jeremy]

We can ship with apps mixing in ActiveJob::TestHelper. Cleaning up the AJ test interaction with queue adapters needs work first.

[rafaelfranca]

@robin850 want to take back on this? If not we should close.

[robin850]

@rafaelfranca : I'm closing this one ; maybe I will work on it later but I don't have much time to spend on it ATM, sorry ! :/