diff options
author | Nobuyoshi Nakada <nobu@ruby-lang.org> | 2021-10-06 11:31:38 +0900 |
---|---|---|
committer | Nobuyoshi Nakada <nobu@ruby-lang.org> | 2021-10-17 16:33:58 +0900 |
commit | 13716898df666210b9067c8a3d05a162c2a6ed66 (patch) | |
tree | 63bf8cb58f5ebc592e5de0bd8295ba680549237e | |
parent | 478187e9a33b7af5b11e570f5133c963af6e1165 (diff) | |
download | ruby-13716898df666210b9067c8a3d05a162c2a6ed66.tar.gz |
Retry hung tests after parallel runs
-rw-r--r-- | tool/lib/test/unit.rb | 70 | ||||
-rw-r--r-- | tool/lib/test/unit/parallel.rb | 4 | ||||
-rw-r--r-- | tool/test/testunit/test_parallel.rb | 13 | ||||
-rw-r--r-- | tool/test/testunit/tests_for_parallel/test4test_hungup.rb | 15 |
4 files changed, 86 insertions, 16 deletions
diff --git a/tool/lib/test/unit.rb b/tool/lib/test/unit.rb index 32694bc27d..6b9bebce62 100644 --- a/tool/lib/test/unit.rb +++ b/tool/lib/test/unit.rb @@ -343,6 +343,7 @@ module Test attr_reader :quit_called attr_accessor :start_time attr_accessor :response_at + attr_accessor :current @@worker_number = 0 @@ -526,7 +527,7 @@ module Test def quit_workers(&cond) return if @workers.empty? - closed = [] + closed = [] if cond @workers.reject! do |worker| next unless cond&.call(worker) begin @@ -536,22 +537,33 @@ module Test rescue Errno::EPIPE rescue Timeout::Error end - closed << worker - worker.close + closed&.push worker + begin + Timeout.timeout(0.2) do + worker.close + end + rescue Timeout::Error + worker.kill + retry + end + @ios.delete worker.io end - return closed if cond - return if @workers.empty? + return if (closed ||= @workers).empty? + pids = closed.map(&:pid) begin - Timeout.timeout(0.2 * @workers.size) do + Timeout.timeout(0.2 * closed.size) do Process.waitall end rescue Timeout::Error - @workers.each do |worker| - worker.kill + if pids + Process.kill(:KILL, *pids) rescue nil + pids = nil + retry end - @workers.clear end + @workers.clear unless cond + closed end FakeClass = Struct.new(:name) @@ -585,6 +597,8 @@ module Test @test_count += 1 jobs_status(worker) + when /^start (.+?)$/ + worker.current = Marshal.load($1.unpack1("m")) when /^done (.+?)$/ begin r = Marshal.load($1.unpack1("m")) @@ -664,7 +678,9 @@ module Test if !(_io = IO.select(@ios, nil, nil, timeout)) timeout = Time.now - @worker_timeout - @tasks.unshift(*quit_workers {|w| w.response_at < timeout}&.map(&:real_file)) + quit_workers {|w| w.response_at < timeout}&.map {|w| + rep << {file: w.real_file, result: nil, testcase: w.current[0], error: w.current} + } elsif _io.first.any? {|io| @need_quit or (deal(io, type, result, rep).nil? and @@ -672,6 +688,7 @@ module Test } break end + break if @tasks.empty? and @workers.empty? if @jobserver and @job_tokens and !@tasks.empty? and ((newjobs = [@tasks.size, @options[:parallel]].min) > @workers.size or !@workers.any? {|x| x.status == :ready}) @@ -707,14 +724,20 @@ module Test unless @interrupt || !@options[:retry] || @need_quit parallel = @options[:parallel] @options[:parallel] = false - suites, rep = rep.partition {|r| r[:testcase] && r[:file] && r[:report].any? {|e| !e[2].is_a?(Test::Unit::PendedError)}} + suites, rep = rep.partition {|r| + r[:testcase] && r[:file] && + (!r.key?(:report) || r[:report].any? {|e| !e[2].is_a?(Test::Unit::PendedError)}) + } suites.map {|r| File.realpath(r[:file])}.uniq.each {|file| require file} - suites.map! {|r| eval("::"+r[:testcase])} del_status_line or puts unless suites.empty? puts "\n""Retrying..." @verbose = options[:verbose] + error, suites = suites.partition {|r| r[:error]} + suites.map! {|r| ::Object.const_get(r[:testcase])} + error.map! {|r| ::Object.const_get(r[:testcase])} _run_suites(suites, type) + result.concat _run_suites(error, type) end @options[:parallel] = parallel end @@ -723,14 +746,21 @@ module Test end unless rep.empty? rep.each do |r| - r[:report].each do |f| + if r[:error] + puke(*r[:error], Timeout::Error) + next + end + r[:report]&.each do |f| puke(*f) if f end end if @options[:retry] - @errors += rep.map{|x| x[:result][0] }.inject(:+) - @failures += rep.map{|x| x[:result][1] }.inject(:+) - @skips += rep.map{|x| x[:result][2] }.inject(:+) + rep.each do |x| + (e, f, s = x[:result]) or next + @errors += e + @failures += f + @skips += s + end end end unless @warnings.empty? @@ -1473,6 +1503,7 @@ module Test assertions = all_test_methods.map { |method| inst = suite.new method + _start_method(inst) inst._assertions = 0 print "#{suite}##{method} = " if @verbose @@ -1494,11 +1525,18 @@ module Test leakchecker.check("#{inst.class}\##{inst.__name__}") end + _end_method(inst) + inst._assertions } return assertions.size, assertions.inject(0) { |sum, n| sum + n } end + def _start_method(inst) + end + def _end_method(inst) + end + ## # Record the result of a single test. Makes it very easy to gather # information. Eg: diff --git a/tool/lib/test/unit/parallel.rb b/tool/lib/test/unit/parallel.rb index db2d918331..b3a8957f26 100644 --- a/tool/lib/test/unit/parallel.rb +++ b/tool/lib/test/unit/parallel.rb @@ -31,6 +31,10 @@ module Test end end + def _start_method(inst) + _report "start", Marshal.dump([inst.class.name, inst.__name__]) + end + def _run_suite(suite, type) # :nodoc: @partial_report = [] orig_testout = Test::Unit::Runner.output diff --git a/tool/test/testunit/test_parallel.rb b/tool/test/testunit/test_parallel.rb index 8207e71868..c1caa3c691 100644 --- a/tool/test/testunit/test_parallel.rb +++ b/tool/test/testunit/test_parallel.rb @@ -49,6 +49,7 @@ module TestParallel assert_match(/^ready/,@worker_out.gets) @worker_in.puts "run #{TESTS}/ptest_first.rb test" assert_match(/^okay/,@worker_out.gets) + assert_match(/^start/,@worker_out.gets) assert_match(/^record/,@worker_out.gets) assert_match(/^p/,@worker_out.gets) assert_match(/^done/,@worker_out.gets) @@ -61,9 +62,11 @@ module TestParallel assert_match(/^ready/,@worker_out.gets) @worker_in.puts "run #{TESTS}/ptest_second.rb test" assert_match(/^okay/,@worker_out.gets) + assert_match(/^start/,@worker_out.gets) assert_match(/^record/,@worker_out.gets) assert_match(/^p/,@worker_out.gets) assert_match(/^done/,@worker_out.gets) + assert_match(/^start/,@worker_out.gets) assert_match(/^record/,@worker_out.gets) assert_match(/^p/,@worker_out.gets) assert_match(/^done/,@worker_out.gets) @@ -76,15 +79,18 @@ module TestParallel assert_match(/^ready/,@worker_out.gets) @worker_in.puts "run #{TESTS}/ptest_first.rb test" assert_match(/^okay/,@worker_out.gets) + assert_match(/^start/,@worker_out.gets) assert_match(/^record/,@worker_out.gets) assert_match(/^p/,@worker_out.gets) assert_match(/^done/,@worker_out.gets) assert_match(/^ready/,@worker_out.gets) @worker_in.puts "run #{TESTS}/ptest_second.rb test" assert_match(/^okay/,@worker_out.gets) + assert_match(/^start/,@worker_out.gets) assert_match(/^record/,@worker_out.gets) assert_match(/^p/,@worker_out.gets) assert_match(/^done/,@worker_out.gets) + assert_match(/^start/,@worker_out.gets) assert_match(/^record/,@worker_out.gets) assert_match(/^p/,@worker_out.gets) assert_match(/^done/,@worker_out.gets) @@ -202,5 +208,12 @@ module TestParallel assert(buf.scan(/^\[\s*\d+\/\d+\]\s*(\d+?)=/).flatten.uniq.size > 1, message("retried tests should run in different processes") {buf}) end + + def test_hungup + spawn_runner "--worker-timeout=1", "test4test_hungup.rb" + buf = Timeout.timeout(TIMEOUT) {@test_out.read} + assert_match(/^Retrying\.+$/, buf) + assert_match(/^2 tests,.* 0 failures,/, buf) + end end end diff --git a/tool/test/testunit/tests_for_parallel/test4test_hungup.rb b/tool/test/testunit/tests_for_parallel/test4test_hungup.rb new file mode 100644 index 0000000000..65a75f7c4d --- /dev/null +++ b/tool/test/testunit/tests_for_parallel/test4test_hungup.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true +require_relative '../../../lib/test/unit' + +class TestHung < Test::Unit::TestCase + def test_success_at_worker + assert true + end + + def test_hungup_at_worker + if on_parallel_worker? + sleep 10 + end + assert true + end +end |