Skip to content

Commit b07362c

Browse files
committed
ActiveSupport::Testing::Isolation: gracefully handle the subprocess dying
Right now if the subprocess exit uncleanly, it straight out bring the parent down with it because it will fail to parse the (likely empty) Marshal payload: ``` <internal:marshal>:34:in `load': marshal data too short (ArgumentError) from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:23:in `run' from 3.3.0+0/gems/minitest-5.20.0/lib/minitest.rb:1094:in `run_one_method' from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:179:in `block in run' from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:168:in `with_timestamps' from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:178:in `run' from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:229:in `block in run_from_queue' from 3.3.0+0/gems/ci-queue-0.38.0/lib/ci/queue/redis/worker.rb:55:in `poll' from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:228:in `run_from_queue' from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:213:in `__run' from 3.3.0+0/gems/minitest-5.20.0/lib/minitest.rb:162:in `run' from 3.3.0+0/gems/minitest-5.20.0/lib/minitest.rb:86:in `block in autorun' - XXXX::XXXXTest#test_xxxxx - /tmp/bundle/ruby/3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:52:in `write': closed stream (IOError) from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:52:in `puts' from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:52:in `block (2 levels) in run_in_isolation' from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:32:in `fork' from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:32:in `block in run_in_isolation' from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:28:in `pipe' from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:28:in `run_in_isolation' from 3.3.0+0/bundler/gems/rails-488a7ce18880/activesupport/lib/active_support/testing/isolation.rb:19:in `run' from 3.3.0+0/gems/minitest-5.20.0/lib/minitest.rb:1094:in `run_one_method' from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:179:in `block in run' from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:168:in `with_timestamps' from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:178:in `run' from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:229:in `block in run_from_queue' from 3.3.0+0/gems/ci-queue-0.38.0/lib/ci/queue/redis/worker.rb:55:in `poll' from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:228:in `run_from_queue' from 3.3.0+0/gems/ci-queue-0.38.0/lib/minitest/queue.rb:213:in `__run' from 3.3.0+0/gems/minitest-5.20.0/lib/minitest.rb:162:in `run' from 3.3.0+0/gems/minitest-5.20.0/lib/minitest.rb:86:in `block in autorun' ``` This breaks the Minitest contract that `run_one_method` shouldn't raise ever, and return a `Minitest::Result`. By properly checking the sub process status, we can turn this crash into a test failure, allowing the original test process to go on.
1 parent 139c567 commit b07362c

File tree

1 file changed

+17
-7
lines changed

1 file changed

+17
-7
lines changed

activesupport/lib/active_support/testing/isolation.rb

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ module Testing
55
module Isolation
66
require "thread"
77

8+
SubprocessCrashed = Class.new(StandardError)
9+
810
def self.included(klass) # :nodoc:
911
klass.class_eval do
1012
parallelize_me!
@@ -16,10 +18,17 @@ def self.forking_env?
1618
end
1719

1820
def run
19-
serialized = run_in_isolation do
21+
status, serialized = run_in_isolation do
2022
super
2123
end
2224

25+
unless status&.success?
26+
error = SubprocessCrashed.new("Subprocess exited with an error: #{status.inspect}\noutput: #{serialized.inspect}")
27+
error.set_backtrace(caller)
28+
self.failures << Minitest::UnexpectedError.new(error)
29+
return defined?(Minitest::Result) ? Minitest::Result.from(self) : dup
30+
end
31+
2332
Marshal.load(serialized)
2433
end
2534

@@ -50,13 +59,13 @@ def run_in_isolation(&blk)
5059
end
5160

5261
write.puts [result].pack("m")
53-
exit!
62+
exit!(0)
5463
end
5564

5665
write.close
5766
result = read.read
58-
Process.wait2(pid)
59-
result.unpack1("m")
67+
_, status = Process.wait2(pid)
68+
return status, result.unpack1("m")
6069
end
6170
end
6271
end
@@ -75,7 +84,7 @@ def run_in_isolation(&blk)
7584
File.open(ENV["ISOLATION_OUTPUT"], "w") do |file|
7685
file.puts [Marshal.dump(test_result)].pack("m")
7786
end
78-
exit!
87+
exit!(0)
7988
else
8089
Tempfile.open("isolation") do |tmpfile|
8190
env = {
@@ -93,13 +102,14 @@ def run_in_isolation(&blk)
93102

94103
child = IO.popen([env, Gem.ruby, *load_path_args, $0, *ORIG_ARGV, test_opts])
95104

105+
status = nil
96106
begin
97-
Process.wait(child.pid)
107+
_, status = Process.wait2(child.pid)
98108
rescue Errno::ECHILD # The child process may exit before we wait
99109
nil
100110
end
101111

102-
return tmpfile.read.unpack1("m")
112+
return status, tmpfile.read.unpack1("m")
103113
end
104114
end
105115
end

0 commit comments

Comments
 (0)