Use an error status instead of nil if we're missing a worker status code #70
Conversation
There was a problem hiding this comment.
This doesn't seem too tough to write a test for, but I'm not sure where to put it. Should we copy one of the suites, say minitest5, and have a suite for test-queue features as opposed to end-to-end tests for each test framework?
A bunch of the minitest5 tests are already like this. It would definitely be nice to break them out to make it clearer that they are not specific to minitest5.
| @@ -150,7 +150,7 @@ def summarize_internal | |||
| worker.suites.size, | |||
| worker.end_time - worker.start_time, | |||
| worker.pid, | |||
| worker.status.exitstatus, | |||
| worker.status.exitstatus || 1, | |||
There was a problem hiding this comment.
It'd be nice to show the signal name if the process was killed by a signal. In fact, it looks like we could replace the whole pid %d exit %d portion of the string with worker.status.to_s, which gives us that same information including info about the signal.
|
There are other uses of |
# Conflicts: # lib/test_queue/runner.rb
|
Okay, I think the only other bad case is covered, and we now output more information when a process dies in an unexpected way. This is ready for another look. |
Workers missing an exit status are an error. This can happen if a worker's process is killed out-of-band. Currently, summarize will explode on the
%call if any of these values are nil.This doesn't seem too tough to write a test for, but I'm not sure where to put it. Should we copy one of the suites, say minitest5, and have a suite for test-queue features as opposed to end-to-end tests for each test framework?