Make the CI Queue reporters work in parallel#355
Conversation
|
Just the python test suite. |
gmcgibbon
force-pushed
the
make_parallel_friendly
branch
4 times, most recently
from
21e1802 to
ba76cc2
Compare
davidstosik
left a comment
davidstosik
left a comment
There was a problem hiding this comment.
Thank you for working on this!
Can we make sure calling file.truncate in #start does not risk truncating the first few logs?
| def start | ||
| @file = File.open(@path, 'w+') | ||
| super | ||
| file.truncate(0) |
There was a problem hiding this comment.
Is #start called only once before forking?
If not, is there a risk that you truncate the few first logs in the case the first tests ran very quickly?
Also, is it okay not to call super?
There was a problem hiding this comment.
Yes it is called once before forking, so the truncate will behave like opening the file in write mode and rewriting sections of it in a non-forked setup.
There was a problem hiding this comment.
Looks like we call https://github.com/minitest/minitest/blob/7c907429e8e7ec7c3b6ee0c7045e67249f4ad505/lib/minitest.rb#L864
Probably not needed, but I see most of the time it is called. I'll add it back just in case.
| unless truffleruby? | ||
| def test_forking |
There was a problem hiding this comment.
Do you mean to run this test only when not running the test suite with Truffle Ruby?
Does that mean the feature implemented in this PR is not compatible with Truffle Ruby?
Do we need a fallback? Will ci-queue not work with Truffle Ruby anymore?
There was a problem hiding this comment.
Fork isn't implemented in truffle ruby. CI queue doesn't fork explicitly anywhere, we're just making sure the logging behaves well when distributed across multiple processes in forks or other threads.
Make Minitest::Queue::OrderReporter parallel-friendly. This is essentially done by appending and truncating to the log file instead of writing to it. The object also doesn't depend on the start hook anymore to initialize the log file, because that isn't called in forked workers.
gmcgibbon
force-pushed
the
make_parallel_friendly
branch
from
ba76cc2 to
52229d6
Compare
4 participants
Make
Minitest::Queue::OrderReporterparallel-friendly. This is essentially done by appending and truncating to the log file instead of writing to it. The object also doesn't depend on the start hook anymore to initialize the log file, because that isn't called in forked workers.