Lazily deserialize job arguments in test helper #39607
Conversation
If there is a job in `enqueued_jobs` whose arguments are not deserializable for some reason (for example, a deleted ActiveRecord model) `assert_enqueued_with` will raise an error even if the job we're looking for is in the `enqueued_jobs` array.
| if expected_args[:job].present? | ||
| if expected_args[:job].respond_to?(:call) | ||
| if !expected_args[:job].call(enqueued_job[:job]) | ||
| potential_matches << enqueued_job | ||
| next | ||
| end | ||
| elsif expected_args[:job] != enqueued_job[:job] | ||
| potential_matches << enqueued_job | ||
| next | ||
| end | ||
| end |
There was a problem hiding this comment.
It seems like this doesn't solve the problem in the general case, since an ActiveJob::DeserializationError can still blow up the assertion if :job isn't specified, no?
Also, I think assert_performed_with needs similar protection. Something like the following would cause the same problem:
model = Model.create!
ModelJob.perform_later(model)
perform_enqueued_jobs
model.destroy
assert_performed_with(job: ModelJob)Perhaps we could rescue ActiveJob::DeserializationError in deserialize_args_for_assertion? Although I'm not clear on whether we should attempt to match such jobs afterwards or not... 🤔
There was a problem hiding this comment.
Yeah - this definitely isn't a "fix all scenarios" patch, just make it less likely to fail 😓
The new scenario you raise is a super interesting one (and solving that might make this a more robust solution). I'll pick this back up later in the week and see what I can come up with.
Thanks for taking a look and giving some feedback, @jonathanhefner!
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
Summary
If there is a job in
enqueued_jobswhose arguments are not deserializable for some reason (for example, a deleted ActiveRecord model)assert_enqueued_withwill raise an error even if the job we're looking for is in theenqueued_jobsarray.This solution isn't as tidy as I was hoping for - definitely open to suggestions on other approaches!
Other Information
Fixes #39606