From c5b78f8bc8fd3431b1ce323a44a6e57ad2836031 Mon Sep 17 00:00:00 2001 From: Clinton Kwarteng Date: Mon, 5 Oct 2026 02:16:52 +0000 Subject: [PATCH 1/3] ruby: Initialize test runs before checking retry expiry --- ruby/lib/ci/queue/static.rb | 2 +- ruby/lib/minitest/queue/runner.rb | 6 +- ruby/test/ci/queue/static_test.rb | 15 +++ ruby/test/integration/minitest_redis_test.rb | 114 +++++++++++++++++-- 4 files changed, 123 insertions(+), 14 deletions(-) diff --git a/ruby/lib/ci/queue/static.rb b/ruby/lib/ci/queue/static.rb index ef00c43..f971872 100644 --- a/ruby/lib/ci/queue/static.rb +++ b/ruby/lib/ci/queue/static.rb @@ -55,7 +55,7 @@ def created_at=(timestamp) end def expired? - (@created_at.to_f TEN_MINUTES) < CI::Queue.time_now.to_f + (@created_at.to_f + TEN_MINUTES) < CI::Queue.time_now.to_f end def populated? diff --git a/ruby/lib/minitest/queue/runner.rb b/ruby/lib/minitest/queue/runner.rb index f030e56..60c4e48 100644 --- a/ruby/lib/minitest/queue/runner.rb +++ b/ruby/lib/minitest/queue/runner.rb @@ -49,6 +49,10 @@ def retry_command def run_command require_worker_id! + # A CI job retry can be the first attempt to reach the test runner. + # Redis uses SETNX here, so an existing run keeps its original age. + queue.rescue_connection_errors { queue.created_at = CI::Queue.time_now.to_f } + if queue.retrying? || retry? if queue.expired? abort! "The test run is too old and can't be retried" @@ -67,8 +71,6 @@ def run_command end end - queue.rescue_connection_errors { queue.created_at = CI::Queue.time_now.to_f } - set_load_path Minitest.queue = queue reporters = [ diff --git a/ruby/test/ci/queue/static_test.rb b/ruby/test/ci/queue/static_test.rb index 2ab40ec..3ab3f58 100644 --- a/ruby/test/ci/queue/static_test.rb +++ b/ruby/test/ci/queue/static_test.rb @@ -16,6 +16,21 @@ def test_shutdown_stops_polling refute_predicate @queue, :exhausted? end + def test_expiry_uses_original_start_time + @queue.created_at = 1000 + @queue.created_at = 2000 + + CI::Queue.stub(:time_now, Time.at(1000)) do + refute_predicate @queue, :expired? + end + CI::Queue.stub(:time_now, Time.at(1600)) do + refute_predicate @queue, :expired? + end + CI::Queue.stub(:time_now, Time.at(1601)) do + assert_predicate @queue, :expired? + end + end + private def test_from_uri diff --git a/ruby/test/integration/minitest_redis_test.rb b/ruby/test/integration/minitest_redis_test.rb index d03803b..b13b99d 100644 --- a/ruby/test/integration/minitest_redis_test.rb +++ b/ruby/test/integration/minitest_redis_test.rb @@ -221,10 +221,10 @@ def test_retry_success end def test_worker_history_retry_replays_all_reserved_tests - run_worker_history_worker(retry_count: 0) + run_retry_worker(retry_count: 0) assert_predicate $?, :success? - out, = run_worker_history_worker(retry_count: 1) + out, = run_retry_worker(retry_count: 1) assert_predicate $?, :success? assert_includes out, 'Replaying 100 tests from worker history.' @@ -233,22 +233,111 @@ def test_worker_history_retry_replays_all_reserved_tests end def test_worker_history_retry_rejoins_queue_without_worker_reservations - run_worker_history_worker(retry_count: 0, build_id: 'missing-history', worker_id: '1') + run_retry_worker(retry_count: 0, build_id: 'missing-history', worker_id: '1') - out, = run_worker_history_worker(retry_count: 1, build_id: 'missing-history', worker_id: '2') + out, = run_retry_worker(retry_count: 1, build_id: 'missing-history', worker_id: '2') assert_predicate $?, :success? assert_includes out, "The retry queue does not contain any failure, we'll process the main queue instead." assert_includes out, 'All tests were ran already' end + def test_buildkite_retry_starts_a_test_run_after_setup_failed + %w[failed-tests worker-history].each do |selection| + [1, 2].each do |retry_count| + build_id = "setup-failure-#{selection}-#{retry_count}" + out, err = run_retry_worker( + retry_count: retry_count, + build_id: build_id, + retry_selection: selection, + extra_args: ['--namespace', 'sinatra-tests'] + ) + + assert_predicate $?, :success?, out + err + assert_includes normalize(out), 'Ran 100 tests, 100 assertions, 0 failures, 0 errors, 0 skips, 0 requeues' + assert_equal 100, @redis.scard("build:sinatra-tests:#{build_id}:processed") + end + end + end + + def test_semaphore_retry_starts_a_test_run_after_setup_failed + out, err = run_retry_worker( + retry_count: 0, + build_id: 'semaphore-setup-failure', + retry_selection: 'failed-tests', + environment: { 'SEMAPHORE_PIPELINE_RERUN' => 'true' } + ) + + assert_predicate $?, :success?, out + err + assert_includes normalize(out), 'Ran 100 tests, 100 assertions, 0 failures, 0 errors, 0 skips, 0 requeues' + end + + def test_buildkite_retry_replays_file_queue + Tempfile.create(['ci-queue-retry', '.txt']) do |file| + file.write((0...100).map { |index| "PassingTest#test_passing_#{index}\n" }.join) + file.flush + + out, err = run_retry_worker( + retry_count: 1, + retry_selection: 'failed-tests', + queue_url: file.path + ) + + assert_predicate $?, :success?, out + err + assert_includes normalize(out), 'Ran 100 tests, 100 assertions, 0 failures, 0 errors, 0 skips, 0 requeues' + end + end + + def test_retry_preserves_existing_test_run_timestamp + %w[failed-tests worker-history].each do |selection| + build_id = "existing-run-#{selection}" + run_retry_worker(retry_count: 0, build_id: build_id) + assert_predicate $?, :success? + created_at = (Time.now.to_f - 3600).to_s + @redis.set("build:#{build_id}:created-at", created_at) + + out, err = run_retry_worker( + retry_count: 1, + build_id: build_id, + retry_selection: selection + ) + + assert_predicate $?, :success?, out + err + assert_equal created_at, @redis.get("build:#{build_id}:created-at") + if selection == 'worker-history' + assert_includes out, 'Replaying 100 tests from worker history.' + else + assert_includes out, 'All tests were ran already' + end + end + end + + def test_retry_rejects_expired_run_without_worker_history + %w[failed-tests worker-history].each do |selection| + build_id = "expired-run-#{selection}" + created_at = (Time.now.to_f - 24 * 60 * 60).to_s + @redis.set("build:#{build_id}:created-at", created_at) + + out, err = run_retry_worker( + retry_count: 1, + build_id: build_id, + retry_selection: selection + ) + + refute_predicate $?, :success?, out + err + assert_includes out, "The test run is too old and can't be retried" + assert_equal created_at, @redis.get("build:#{build_id}:created-at") + assert_equal 0, @redis.scard("build:#{build_id}:processed") + end + end + def test_worker_history_retry_fails_when_replay_stops_early - run_worker_history_worker( + run_retry_worker( retry_count: 0, build_id: 'incomplete-history', test_file: 'test/failing_test.rb' ) - out, err = run_worker_history_worker( + out, err = run_retry_worker( retry_count: 1, build_id: 'incomplete-history', test_file: 'test/failing_test.rb', @@ -827,28 +916,31 @@ def test_utf8_tests_and_marshal private - def run_worker_history_worker( + def run_retry_worker( retry_count:, build_id: 'worker-history', worker_id: '1', test_file: 'test/passing_test.rb', - extra_args: [] + extra_args: [], + retry_selection: 'worker-history', + environment: {}, + queue_url: @redis_url ) args = [ @exe, 'run', - '--queue', @redis_url, + '--queue', queue_url, '--seed', 'foobar', '--build', build_id, '--worker', worker_id, '--timeout', '1', - '--retry-selection', 'worker-history' + '--retry-selection', retry_selection ] args.concat(extra_args) args.push('-Itest', test_file) capture_subprocess_io do system( - { 'BUILDKITE_RETRY_COUNT' => retry_count.to_s }, + { 'BUILDKITE_RETRY_COUNT' => retry_count.to_s }.merge(environment), *args, chdir: 'test/fixtures/' ) From 60e0d33efc6bd5f359eb153a3c739b5c142030d5 Mon Sep 17 00:00:00 2001 From: Clinton Kwarteng Date: Mon, 5 Oct 2026 02:18:56 +0000 Subject: [PATCH 2/3] ruby: Require tempfile explicitly in retry integration tests --- ruby/test/integration/minitest_redis_test.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/ruby/test/integration/minitest_redis_test.rb b/ruby/test/integration/minitest_redis_test.rb index b13b99d..ebe5b07 100644 --- a/ruby/test/integration/minitest_redis_test.rb +++ b/ruby/test/integration/minitest_redis_test.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true require 'test_helper' +require 'tempfile' require 'tmpdir' require 'active_support/testing/time_helpers' require 'concurrent/set' From d569aa572c544d895b835735be3563a6946cf62e Mon Sep 17 00:00:00 2001 From: Clinton Kwarteng Date: Mon, 5 Oct 2026 17:23:40 +0000 Subject: [PATCH 3/3] ruby: Keep the legacy ActiveSupport test fixture on JSON 2 --- ruby/Gemfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ruby/Gemfile b/ruby/Gemfile index 6af3554..bef7daf 100644 --- a/ruby/Gemfile +++ b/ruby/Gemfile @@ -4,4 +4,6 @@ source 'https://rubygems.org' gemspec gem 'activesupport', '~> 5.2.0' +# ActiveSupport 5.2's JSON encoder passes quirks_mode, removed in JSON 3. +gem 'json', '< 3' gem 'rexml'