diff --git a/ruby/lib/ci/queue/redis/supervisor.rb b/ruby/lib/ci/queue/redis/supervisor.rb index 96e796ce..0fcc47d5 100644 --- a/ruby/lib/ci/queue/redis/supervisor.rb +++ b/ruby/lib/ci/queue/redis/supervisor.rb @@ -24,41 +24,28 @@ def wait_for_workers yield if block_given? - time_left = config.report_timeout - duration.to_i - time_left_with_no_workers = config.inactive_workers_timeout - until exhausted? || time_left <= 0 || max_test_failed? || time_left_with_no_workers <= 0 - time_left -= 1 + @time_left = config.report_timeout - duration.to_i + @time_left_with_no_workers = config.inactive_workers_timeout + until exhausted? || @time_left <= 0 || max_test_failed? || @time_left_with_no_workers <= 0 + @time_left -= 1 sleep 1 if active_workers? - time_left_with_no_workers = config.inactive_workers_timeout + @time_left_with_no_workers = config.inactive_workers_timeout else - time_left_with_no_workers -= 1 + @time_left_with_no_workers -= 1 end yield if block_given? end - if time_left <= 0 && !exhausted? - puts "Aborting, timed out." - - remaining_tests = test_ids - remaining_tests.first(10).each do |id| - puts " #{id}" - end - - if remaining_tests.size > 10 - puts " ..." - end - end - - puts "Aborting, it seems all workers died." if time_left_with_no_workers <= 0 && !exhausted? - exhausted? rescue CI::Queue::Redis::LostMaster false end + attr_reader :time_left, :time_left_with_no_workers + private def active_workers? diff --git a/ruby/lib/minitest/queue/build_status_reporter.rb b/ruby/lib/minitest/queue/build_status_reporter.rb index b34084cb..cd947539 100644 --- a/ruby/lib/minitest/queue/build_status_reporter.rb +++ b/ruby/lib/minitest/queue/build_status_reporter.rb @@ -4,8 +4,9 @@ module Queue class BuildStatusReporter < Minitest::Reporters::BaseReporter include ::CI::Queue::OutputHelpers - def initialize(build:, **options) - @build = build + def initialize(supervisor:, **options) + @supervisor = supervisor + @build = supervisor.build super(options) end @@ -27,6 +28,23 @@ def requeued_tests def report puts aggregates + + if supervisor.time_left.to_i <= 0 + puts "Timed out waiting for tests to be executed." + + remaining_tests = supervisor.test_ids + remaining_tests.first(10).each do |id| + puts " #{id}" + end + + if remaining_tests.size > 10 + puts " ..." + end + end + + puts "All workers died." if supervisor.time_left_with_no_workers.to_i <= 0 + puts 'Encountered too many failed tests. Test run was ended early.' if supervisor.max_test_failed? + errors = error_reports puts errors @@ -92,7 +110,7 @@ def write_flaky_tests_file(file) private - attr_reader :build + attr_reader :build, :supervisor def aggregates success = failures.zero? && errors.zero? diff --git a/ruby/lib/minitest/queue/runner.rb b/ruby/lib/minitest/queue/runner.rb index 5b437088..31183a82 100644 --- a/ruby/lib/minitest/queue/runner.rb +++ b/ruby/lib/minitest/queue/runner.rb @@ -257,23 +257,16 @@ def report_command end unless supervisor.exhausted? - reporter = BuildStatusReporter.new(build: supervisor.build) + reporter = BuildStatusReporter.new(supervisor: supervisor) reporter.report reporter.write_failure_file(queue_config.failure_file) if queue_config.failure_file reporter.write_flaky_tests_file(queue_config.export_flaky_tests_file) if queue_config.export_flaky_tests_file - msg = "#{supervisor.size} tests weren't run." - - if supervisor.max_test_failed? - puts('Encountered too many failed tests. Test run was ended early.') - abort!(msg) - else - abort!(msg) - end + abort!("#{supervisor.size} tests weren't run.") end end - reporter = BuildStatusReporter.new(build: supervisor.build) + reporter = BuildStatusReporter.new(supervisor: supervisor) reporter.write_failure_file(queue_config.failure_file) if queue_config.failure_file reporter.write_flaky_tests_file(queue_config.export_flaky_tests_file) if queue_config.export_flaky_tests_file reporter.report diff --git a/ruby/test/ci/queue/redis_supervisor_test.rb b/ruby/test/ci/queue/redis_supervisor_test.rb index 14ed41f4..f46baa31 100644 --- a/ruby/test/ci/queue/redis_supervisor_test.rb +++ b/ruby/test/ci/queue/redis_supervisor_test.rb @@ -39,18 +39,6 @@ def test_wait_for_workers assert_equal true, workers_done end - def test_wait_for_workers_timeout - @supervisor = supervisor(timeout: 10, queue_init_timeout: 0.1) - io = nil - thread = Thread.start do - io = capture_io { @supervisor.wait_for_workers } - end - thread.wakeup - worker(1) - thread.join - assert_includes io.join, "Aborting, it seems all workers died.\n" - end - def test_num_workers assert_equal 0, @supervisor.workers_count worker(1) diff --git a/ruby/test/integration/minitest_redis_test.rb b/ruby/test/integration/minitest_redis_test.rb index 1207ed09..104cf351 100644 --- a/ruby/test/integration/minitest_redis_test.rb +++ b/ruby/test/integration/minitest_redis_test.rb @@ -237,9 +237,35 @@ def test_max_test_failed assert_equal expected.strip, normalize(out.lines[0..2].join.strip) expected = <<~EXPECTED Encountered too many failed tests. Test run was ended early. + EXPECTED + assert_equal expected.strip, normalize(out.lines[3].strip) + expected = <<~EXPECTED 97 tests weren't run. EXPECTED - assert_equal expected.strip, normalize(out.lines.last(2).join.strip) + assert_equal expected.strip, normalize(out.lines.last.strip) + end + + def test_all_workers_died + # Run the reporter + out, err = capture_subprocess_io do + system( + @exe, 'report', + '--queue', @redis_url, + '--seed', 'foobar', + '--build', '1', + '--timeout', '1', + '--max-test-failed', '3', + chdir: 'test/fixtures/', + ) + end + + refute_predicate $?, :success? + assert_empty err + expected = <<~EXPECTED + Waiting for workers to complete + No master was elected. Did all workers crash? + EXPECTED + assert_equal expected.strip, normalize(out.lines[0..2].join.strip) end def test_circuit_breaker diff --git a/ruby/test/minitest/queue/build_status_recorder_test.rb b/ruby/test/minitest/queue/build_status_recorder_test.rb index a2cf52e0..7dff48a1 100644 --- a/ruby/test/minitest/queue/build_status_recorder_test.rb +++ b/ruby/test/minitest/queue/build_status_recorder_test.rb @@ -65,7 +65,7 @@ def worker(id) end def summary - @summary ||= Minitest::Queue::BuildStatusReporter.new(build: @queue.supervisor.build) + @summary ||= Minitest::Queue::BuildStatusReporter.new(supervisor: @queue.supervisor) end end end