ruby-changes:68512
From: Nobuyoshi <ko1@a...>
Date: Sun, 17 Oct 2021 16:34:25 +0900 (JST)
Subject: [ruby-changes:68512] 13716898df (master): Retry hung tests after parallel runs
https://git.ruby-lang.org/ruby.git/commit/?id=13716898df From 13716898df666210b9067c8a3d05a162c2a6ed66 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada <nobu@r...> Date: Wed, 6 Oct 2021 11:31:38 +0900 Subject: Retry hung tests after parallel runs --- tool/lib/test/unit.rb | 70 +++++++++++++++++----- tool/lib/test/unit/parallel.rb | 4 ++ tool/test/testunit/test_parallel.rb | 13 ++++ .../tests_for_parallel/test4test_hungup.rb | 15 +++++ 4 files changed, 86 insertions(+), 16 deletions(-) create mode 100644 tool/test/testunit/tests_for_parallel/test4test_hungup.rb 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 https://github.com/ruby/ruby/blob/trunk/tool/lib/test/unit.rb#L343 attr_reader :quit_called attr_accessor :start_time attr_accessor :response_at + attr_accessor :current @@worker_number = 0 @@ -526,7 +527,7 @@ module Test https://github.com/ruby/ruby/blob/trunk/tool/lib/test/unit.rb#L527 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 https://github.com/ruby/ruby/blob/trunk/tool/lib/test/unit.rb#L537 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 https://github.com/ruby/ruby/blob/trunk/tool/lib/test/unit.rb#L597 @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 https://github.com/ruby/ruby/blob/trunk/tool/lib/test/unit.rb#L678 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 https://github.com/ruby/ruby/blob/trunk/tool/lib/test/unit.rb#L688 } 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 https://github.com/ruby/ruby/blob/trunk/tool/lib/test/unit.rb#L724 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 https://github.com/ruby/ruby/blob/trunk/tool/lib/test/unit.rb#L746 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 https://github.com/ruby/ruby/blob/trunk/tool/lib/test/unit.rb#L1503 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 https://github.com/ruby/ruby/blob/trunk/tool/lib/test/unit.rb#L1525 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 https://github.com/ruby/ruby/blob/trunk/tool/lib/test/unit/parallel.rb#L31 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 https://github.com/ruby/ruby/blob/trunk/tool/test/testunit/test_parallel.rb#L49 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 https://github.com/ruby/ruby/blob/trunk/tool/test/testunit/test_parallel.rb#L62 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 https://github.com/ruby/ruby/blob/trunk/tool/test/testunit/test_parallel.rb#L79 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 https://github.com/ruby/ruby/blob/trunk/tool/test/testunit/test_parallel.rb#L208 assert(buf.scan(/^\[\s*\d+\/\d+\]\s*(\d+?)=/).flatten.uniq (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/