mirror of https://github.com/rails/rails
Fix ActiveJob Test adapter not respecting retry attempts:
- ### Problem Given the below example the test adapter will retry the job indefinitely: ```ruby class BuggyJob < ActiveJob::Base retry_on(Exception, attempts: 2) def perform raise "error" end end BuggyJob.perform_later perform_enqueued_jobs ``` The problem is that when the job get retried, the `exception_executions` variable is not serialized/deserialized, resulting in ActiveJob to not be able to determine how many time this job was retried. The solution in this PR is to deserialize the whole job in the test adapter, and reserialize it before retrying. Fix #38391
This commit is contained in:
parent
5613b37b26
commit
d35cf4c05d
|
@ -41,7 +41,11 @@ module ActiveJob
|
|||
|
||||
private
|
||||
def job_to_hash(job, extras = {})
|
||||
{ job: job.class, args: job.serialize.fetch("arguments"), queue: job.queue_name }.merge!(extras)
|
||||
job.serialize.tap do |job_data|
|
||||
job_data[:job] = job.class
|
||||
job_data[:args] = job_data.fetch("arguments")
|
||||
job_data[:queue] = job_data.fetch("queue_name")
|
||||
end.merge(extras)
|
||||
end
|
||||
|
||||
def perform_or_enqueue(perform, job, job_data)
|
||||
|
|
|
@ -674,10 +674,9 @@ module ActiveJob
|
|||
end
|
||||
|
||||
def instantiate_job(payload)
|
||||
args = ActiveJob::Arguments.deserialize(payload[:args])
|
||||
job = payload[:job].new(*args)
|
||||
job = payload[:job].deserialize(payload)
|
||||
job.scheduled_at = Time.at(payload[:at]) if payload.key?(:at)
|
||||
job.queue_name = payload[:queue]
|
||||
job.send(:deserialize_arguments_if_needed)
|
||||
job
|
||||
end
|
||||
|
||||
|
|
|
@ -7,6 +7,7 @@ require "jobs/hello_job"
|
|||
require "jobs/logging_job"
|
||||
require "jobs/nested_job"
|
||||
require "jobs/rescue_job"
|
||||
require "jobs/raising_job"
|
||||
require "jobs/inherited_job"
|
||||
require "jobs/multiple_kwargs_job"
|
||||
require "models/person"
|
||||
|
@ -660,7 +661,7 @@ class EnqueuedJobsTest < ActiveJob::TestCase
|
|||
end
|
||||
end
|
||||
assert_match(/No enqueued job found with {:job=>HelloJob, :args=>\[#{wilma.inspect}\]}/, error.message)
|
||||
assert_match(/Potential matches: {:job=>HelloJob, :args=>\[#<Person.* @id=\"9\"\>\], :queue=>\"default\"}/, error.message)
|
||||
assert_match(/Potential matches: {.*?:job=>HelloJob, :args=>\[#<Person.* @id=\"9\"\>\], :queue=>\"default\"}/, error.message)
|
||||
end
|
||||
|
||||
def test_assert_enqueued_with_failure_with_no_block_with_global_id_args
|
||||
|
@ -672,7 +673,7 @@ class EnqueuedJobsTest < ActiveJob::TestCase
|
|||
end
|
||||
|
||||
assert_match(/No enqueued job found with {:job=>HelloJob, :args=>\[#{wilma.inspect}\]}/, error.message)
|
||||
assert_match(/Potential matches: {:job=>HelloJob, :args=>\[#<Person.* @id=\"9\"\>\], :queue=>\"default\"}/, error.message)
|
||||
assert_match(/Potential matches: {.*?:job=>HelloJob, :args=>\[#<Person.* @id=\"9\"\>\], :queue=>\"default\"}/, error.message)
|
||||
end
|
||||
|
||||
def test_assert_enqueued_with_does_not_change_jobs_count
|
||||
|
@ -1825,7 +1826,7 @@ class PerformedJobsTest < ActiveJob::TestCase
|
|||
end
|
||||
end
|
||||
assert_match(/No performed job found with {:job=>HelloJob, :args=>\[#{wilma.inspect}\]}/, error.message)
|
||||
assert_match(/Potential matches: {:job=>HelloJob, :args=>\[#<Person.* @id=\"9\"\>\], :queue=>\"default\"}/, error.message)
|
||||
assert_match(/Potential matches: {.*?:job=>HelloJob, :args=>\[#<Person.* @id=\"9\"\>\], :queue=>\"default\"}/, error.message)
|
||||
end
|
||||
|
||||
def test_assert_performed_with_without_block_failure_with_global_id_args
|
||||
|
@ -1838,7 +1839,7 @@ class PerformedJobsTest < ActiveJob::TestCase
|
|||
end
|
||||
|
||||
assert_match(/No performed job found with {:job=>HelloJob, :args=>\[#{wilma.inspect}\]}/, error.message)
|
||||
assert_match(/Potential matches: {:job=>HelloJob, :args=>\[#<Person.* @id=\"9\"\>\], :queue=>\"default\"}/, error.message)
|
||||
assert_match(/Potential matches: {.*?:job=>HelloJob, :args=>\[#<Person.* @id=\"9\"\>\], :queue=>\"default\"}/, error.message)
|
||||
end
|
||||
|
||||
def test_assert_performed_with_does_not_change_jobs_count
|
||||
|
@ -1864,6 +1865,16 @@ class PerformedJobsTest < ActiveJob::TestCase
|
|||
|
||||
assert_equal 2, queue_adapter.performed_jobs.count
|
||||
end
|
||||
|
||||
test "TestAdapter respect max attempts" do
|
||||
RaisingJob.perform_later
|
||||
|
||||
assert_raises(RaisingJob::MyError) do
|
||||
perform_enqueued_jobs
|
||||
end
|
||||
|
||||
assert_equal 2, queue_adapter.enqueued_jobs.count
|
||||
end
|
||||
end
|
||||
|
||||
class OverrideQueueAdapterTest < ActiveJob::TestCase
|
||||
|
|
|
@ -0,0 +1,11 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class RaisingJob < ActiveJob::Base
|
||||
MyError = Class.new(StandardError)
|
||||
|
||||
retry_on(MyError, attempts: 2)
|
||||
|
||||
def perform
|
||||
raise MyError
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue