From 4b708c5ca6c31315158579cac230de43cd27bed4 Mon Sep 17 00:00:00 2001 From: wintan1418 Date: Thu, 30 Jul 2026 18:56:27 +0100 Subject: [PATCH 1/3] Give in-flight jobs enough pause to survive slow shutdowns in tests Three tests assert that a job paused mid-flight does NOT complete while its process is terminated: the async and forked "term supervisor exceeding timeout" tests and "don't block claimed executions that get released". They paused the job for shutdown_timeout + 10 seconds, but that clock starts when the job starts executing, and on a slow CI runner the test's detection waits (up to 8 seconds of polling for the job to show as started) plus the termination window (up to 7 more) can outlast it: the sleep finishes inside the still-alive process, the job writes its completed status, and the assertion fails. Widen the pause to shutdown_timeout + 30 seconds. The job is killed mid-sleep either way, so the test's runtime doesn't change; only the margin against slow runners does. Related to rails/solid_queue#602 --- test/integration/async_processes_lifecycle_test.rb | 2 +- test/integration/concurrency_controls_test.rb | 2 +- test/integration/forked_processes_lifecycle_test.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/integration/async_processes_lifecycle_test.rb b/test/integration/async_processes_lifecycle_test.rb index 7f2e7007..f1aa9c4a 100644 --- a/test/integration/async_processes_lifecycle_test.rb +++ b/test/integration/async_processes_lifecycle_test.rb @@ -128,7 +128,7 @@ class AsyncProcessesLifecycleTest < ActiveSupport::TestCase test "term supervisor exceeding timeout while there are jobs in-flight" do no_pause = enqueue_store_result_job("no pause") - pause = enqueue_store_result_job("pause", pause: SolidQueue.shutdown_timeout + 10.second) + pause = enqueue_store_result_job("pause", pause: SolidQueue.shutdown_timeout + 30.seconds) # Wait for the "no pause" job to complete and the pause job to start. # A claimed execution alone is not enough here because the worker may have diff --git a/test/integration/concurrency_controls_test.rb b/test/integration/concurrency_controls_test.rb index 9cdb3d65..0f20034c 100644 --- a/test/integration/concurrency_controls_test.rb +++ b/test/integration/concurrency_controls_test.rb @@ -174,7 +174,7 @@ class ConcurrencyControlsTest < ActiveSupport::TestCase end test "don't block claimed executions that get released" do - NonOverlappingUpdateResultJob.perform_later(@result, name: "I'll be released to ready", pause: SolidQueue.shutdown_timeout + 10.seconds) + NonOverlappingUpdateResultJob.perform_later(@result, name: "I'll be released to ready", pause: SolidQueue.shutdown_timeout + 30.seconds) job = SolidQueue::Job.last wait_for(timeout: 2.seconds) { job.reload.claimed? } diff --git a/test/integration/forked_processes_lifecycle_test.rb b/test/integration/forked_processes_lifecycle_test.rb index deb196d3..90317701 100644 --- a/test/integration/forked_processes_lifecycle_test.rb +++ b/test/integration/forked_processes_lifecycle_test.rb @@ -124,7 +124,7 @@ class ForkedProcessesLifecycleTest < ActiveSupport::TestCase test "term supervisor exceeding timeout while there are jobs in-flight" do no_pause = enqueue_store_result_job("no pause") - pause = enqueue_store_result_job("pause", pause: SolidQueue.shutdown_timeout + 10.seconds) + pause = enqueue_store_result_job("pause", pause: SolidQueue.shutdown_timeout + 30.seconds) wait_while_with_timeout(5.seconds) { SolidQueue::ReadyExecution.joins(:job).exists?(solid_queue_jobs: { active_job_id: pause.job_id }) From a3a17176d38f0bbb87b26755885ed23c6dbdb5b8 Mon Sep 17 00:00:00 2001 From: wintan1418 Date: Thu, 30 Jul 2026 19:49:09 +0100 Subject: [PATCH 2/3] Add forensics to the pause-job assertion in the async lifecycle test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pause job's sleep is far longer than the test's entire timeline, so when this assertion fails on CI with the job completed, the write must have come from outside the test's own flow — a process or thread leaked from a neighboring test. That can't be diagnosed from "expected completed to not be equal to completed", so include all job results and the registered processes in the failure message. --- test/integration/async_processes_lifecycle_test.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/integration/async_processes_lifecycle_test.rb b/test/integration/async_processes_lifecycle_test.rb index f1aa9c4a..65a17ad6 100644 --- a/test/integration/async_processes_lifecycle_test.rb +++ b/test/integration/async_processes_lifecycle_test.rb @@ -153,8 +153,15 @@ class AsyncProcessesLifecycleTest < ActiveSupport::TestCase assert_completed_job_results("no pause") assert_job_status(no_pause, :finished) - # The pause job should not have completed - assert_not_equal "completed", skip_active_record_query_cache { JobResult.find_by(value: "pause")&.status } + # The pause job should not have completed. Its pause is far longer than this + # test's entire timeline, so a completed result can only come from outside + # the test's own flow — include enough state to tell where it came from. + skip_active_record_query_cache do + assert_not_equal "completed", JobResult.find_by(value: "pause")&.status, + "Expected the pause job not to complete. " \ + "Job results: #{JobResult.all.map { |r| { id: r.id, queue: r.queue_name, status: r.status, value: r.value, updated_at: r.updated_at } }.inspect}; " \ + "registered processes: #{SolidQueue::Process.all.map { |p| { id: p.id, kind: p.kind, pid: p.pid, last_heartbeat_at: p.last_heartbeat_at } }.inspect}" + end # After shutdown, the pause job may be either: # - claimed (exit! called, no cleanup) OR From 960e5a9847b26e177b475bdb7777178bf4853926 Mon Sep 17 00:00:00 2001 From: wintan1418 Date: Thu, 30 Jul 2026 20:09:45 +0100 Subject: [PATCH 3/3] Wait reliably for the killed job to start in "kill worker individually" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test waits for the job's "started" result before SIGKILLing the worker, but with the non-raising wait and only 2 seconds of margin: on a slow runner, worker boot plus polling plus claiming can take longer, the guard gives up silently, and the KILL lands before the job ever starts — so the "started" row the test asserts on later never gets written. Use the raising variant with a 10 second margin, so a job that never starts fails the test right at the guard, and widen the job's pause so the KILL can't arrive after the pause has already finished on a slow runner (which would complete the job and erase the "started" status too). Neither change affects the test's runtime in the intended flow. --- test/integration/forked_processes_lifecycle_test.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/integration/forked_processes_lifecycle_test.rb b/test/integration/forked_processes_lifecycle_test.rb index 90317701..036ba239 100644 --- a/test/integration/forked_processes_lifecycle_test.rb +++ b/test/integration/forked_processes_lifecycle_test.rb @@ -238,12 +238,15 @@ class ForkedProcessesLifecycleTest < ActiveSupport::TestCase end test "kill worker individually" do - killed_pause = enqueue_store_result_job("killed_pause", pause: 2.seconds) + killed_pause = enqueue_store_result_job("killed_pause", pause: 5.seconds) enqueue_store_result_job("pause", :default, pause: 0.5.seconds) wait_for_jobs_to_finish_for(1.second, except: [ killed_pause ]) - # Ensure the long job has written its "started" row before we SIGKILL the worker. - wait_while_with_timeout(2.seconds) do + # Ensure the long job has written its "started" row before we SIGKILL the + # worker. Raise if it never does: proceeding would kill the worker before + # it starts the job, so no "started" row would ever exist and the test + # would fail later, in a way much harder to trace back here. + wait_while_with_timeout!(10.seconds) do JobResult.where(status: "started", value: "killed_pause").none? end