Permalink
Browse files

UnexpectedErrors may reference exceptions that can't be dumped

UnexpectedError exceptions wrap the original exception, and the original
exception may contain a reference to something that can't be marshal
dumped which will cause the process to die.
  • Loading branch information...
1 parent c6af3ba commit efb835c9c0aea9d2a6487a6fdd31d459f1771879 @tenderlove tenderlove committed Aug 13, 2014
Showing with 17 additions and 0 deletions.
  1. +17 −0 actionpack/test/abstract_unit.rb
@@ -484,13 +484,30 @@ def shutdown
method = job[1]
reporter = job[2]
result = Minitest.run_one_method klass, method
+ if result.error?
+ translate_exceptions result
+ end
queue.record reporter, result
end
}
}
@size.times { @queue << nil }
pool.each { |pid| Process.waitpid pid }
end
+
+ private
+ def translate_exceptions(result)
+ result.failures.map! { |e|
@mrageh

mrageh Dec 9, 2014

Contributor

Hi @tenderlove
Is there a specific reason for using {} instead of do...end in the block beginning on line 500?
😺

+ begin
+ Marshal.dump e
+ e
+ rescue TypeError
+ ex = Exception.new e.message
+ ex.set_backtrace e.backtrace
+ Minitest::UnexpectedError.new ex
+ end
+ }
+ end
end
if ActiveSupport::Testing::Isolation.forking_env? && PROCESS_COUNT > 0

2 comments on commit efb835c

Owner

jeremy replied Aug 13, 2014

Is this specific to the parallel test runner? Trying to think when else we're shipping exceptions over the wire.

Owner

tenderlove replied Aug 13, 2014

@jeremy this issue is specific to the forking test runner. AFAICT, UnexpectedError exceptions are the only time we would attempt to marshal unmarshallable things. I filed a ticket about this since minitest implements marshal_dump so that tests can be marshaled.

Please sign in to comment.