fix(minitest): stop discarding stdout written by the tests themselves - #105
Closed
rsanheim wants to merge 2 commits into
Closed
fix(minitest): stop discarding stdout written by the tests themselves#105rsanheim wants to merge 2 commits into
rsanheim wants to merge 2 commits into
Conversation
Minitest writes progress characters without a trailing newline, so a `puts`
from inside a test lands as a tail on the current progress line ("..in
test_foo"). The parser extracted the leading progress characters and left the
whole line unconsumed, which sent it to the collector's rawOutput buffer - and
that buffer is only ever printed for failed or errored workers. On a green run
every bit of test output was dropped on the floor.
Split the two streams in the parser instead: leading progress characters feed
the aggregated progress line, the remaining text becomes a TestStdout
notification the collector buffers separately. PrintResults flushes it after
the progress line is complete, so the consolidated dot line stays intact across
parallel workers. This mirrors how RSpec already separates its JSON protocol
lines from arbitrary test stdout.
The split runs as the last check in ParseLine so minitest's own output -
failure headers, the timing line, the summary - is always claimed first.
…ing one The collector appended "\n" to every captured fragment. For a `puts` that happens to be right - the reader stripped a real newline and this put it back - but it is the wrong rule: a fragment from a line with no trailing newline got one it never had. ReadString only returns a nil error when it actually found the delimiter, so stream_helper is the last place that knows. Carry the terminator from there and write the content verbatim downstream. This does not change how a `puts` renders. It stops plur from manufacturing output the test did not write.
Owner
Author
|
Superseded by #106. This fixed the dropped output, but with heuristics layered on the existing guess: it kept #106 removes the ambiguity instead of coping with it, by injecting a minitest plugin that tags test-written stdout — the same mechanism plur already uses for RSpec's formatter, with the marking inverted so bare progress characters stay free. That let the heuristics be deleted outright. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In minitest mode, plur silently discarded stdout written by the tests themselves. A
putsinside a test never reached the user on a passing run.Reproducing
fixtures/projects/minitest-successalready containsputs "in test_addition"andputs "in test_titleize". Before this change:Neither string appears anywhere in stdout or stderr. After:
Root cause
Minitest writes progress characters with no trailing newline, so a
putsfrom inside a test lands as a tail on the current progress line —..in test_foo.The parser pulled the leading progress characters off that line but returned
consumed = false, so the whole line went to the collector'srawOutputbuffer. That buffer is only ever printed for failed or errored workers (result.go), gated further upstream bystreamTestOutput'sstreamStdoutparameter, whichrunner.gopasses asFramework.Name != "minitest". On a green run there was no code path that printed it.Worth noting the text was never lost inside the parser — on a failing run the raw dump has always contained it. The defect was purely that nothing printed it when the run passed.
The fix
Split the two streams in the parser: leading progress characters feed the aggregated progress line, and the remaining text becomes a
TestStdoutnotification the collector buffers separately.PrintResultsflushes it once the progress line is complete, so the consolidated dot line stays intact across parallel workers. This mirrors how the RSpec path already separates its JSON protocol lines from arbitrary test stdout.The split is deliberately the last check in
ParseLine, so minitest's own output — failure headers, the timing line, the summary — is always claimed first. Placing it earlier causes the summary line to be swallowed as test output andSuiteFinishednever fires; two existing unit tests cover that ordering.Behavior notes
puts, the consolidated progress line is unchanged: the first stdout line is still exactly the run's progress characters at both-n 1and-n 2.putsoutput now appears once rather than being embedded in the raw failure dump twice.Run options: --seed Nstill only shows on failing runs. Left as-is here; making the seed visible for reproducing order-dependent failures is a reasonable follow-up.Verification
go test ./...: all packages ok.PLUR_RACE=1 bin/rake test:goclean, with the race banner confirmed present rather than assumed.spec/integration/spec/minitest_integration_spec.rb: 10 examples, 0 failures. No existing example was modified or weakened; the 2-worker example gained assertions.Known limitation:
printwithout a trailing newlinecountLeadingProgressCharsonly claims progress characters at the start of a line. Minitest packs dots and test output onto one physical line, so withprint(no trailing newline) every dot appearing after the first chunk of text is treated as literal text.Given tests doing
print "AAA"; print "BBB"andprint "CCC", raw minitest emits one line with all 8 dots interleaved:plur renders a 1-character progress line and carries the remaining 7 dots inside the text:
This is pre-existing — a pre-fix binary also shows a 1-dot progress line — and this PR does not make the dot count any worse. It does make the mangling visible, where before the text was silently discarded.
puts, which is the overwhelmingly common case, is unaffected because each write is newline-terminated.Properly fixing this needs the structured reporter described below, not more heuristics: stripping all progress characters from a line would eat a test that legitimately prints
....Known limitation, pre-existing
countLeadingProgressCharstreats a leadingF/E/Sin test output as a progress character, soputs "FIRST_LINE"printsIRST_LINEand injects a spuriousFinto the progress line. This predates the change — a pre-fix binary already emits the bogusF— and this PR only makes the truncated remainder visible. Distinguishing "F for a failed test" from "F starting a line of test output" is ambiguous in a line-based parse; the principled fix is a structured minitest reporter mirroringinternal/framework/rspec/formatter.rb. Tracked separately.