Deliver the interpreter output of tests to RunTests#1205
Open
Donach wants to merge 1 commit into
Open
Conversation
RunTests.redirectInterpreterOutput never received anything, so whatever a test printed went to System.err instead of the stream RunTests writes to. As a consequence -compactOutput could not suppress that output either. Three independent reasons, all fixed here: - ProgramStateIO shadowed ProgramState.outStream with its own field and overrode getOutStream/setOutStream without forwarding to the native providers. Removed, the base class already implements both correctly. - ReflectionNativeProvider.setOutStream was an empty method, so OutputProvider, which implements println/BJDebugMsg/DisplayTextToPlayer and friends, kept writing to its default System.err. - The PrintStream wrapping the redirect had no autoflush, so its buffer was never emptied. It now autoflushes and uses an explicit UTF-8 charset on both the encoding and the decoding side.
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.
RunTests.redirectInterpreterOutputinstalls a stream so that whatever the tests printgoes to
RunTests' own output. That stream never received anything: the output wentstraight to
System.err, bypassingRunTestsentirely. A side effect is that-compactOutputcannot suppress it either, so it still shows up in CI logs.Three independent reasons, each fixed here:
ProgramStateIOshadows the stream. It declares its ownprivate PrintStream outStreamand overridesgetOutStream/setOutStreamon top ofProgramState, whichalready has both. The base implementation forwards the new stream to every registered
NativesProvider; the override did not, so the providers kept their old stream. Thefield and the two overrides are removed and the base class does the work.
ReflectionNativeProvider.setOutStreamis empty. It is the provider that ownsOutputProvider, which implementsprintln,BJDebugMsg,DisplayTextToPlayerand theother printing natives. Because the setter did nothing,
OutputProviderkept itsdefault
System.err. It now forwards to theOutputProviderinstances it holds, andOutputProvidergets the corresponding setter.The
PrintStreamhas no autoflush. Even with the two fixes above, output sat in the8 KB buffer of
new PrintStream(os)and was never flushed, so a normal test rundelivered nothing. It now autoflushes, with an explicit UTF-8 charset on the encoding
side and the matching
new String(bytes, UTF_8)on the decoding side, which also fixesnon-ASCII output being decoded with the platform default charset.
Effect
Output printed by tests during
-runtestsnow reaches the streamRunTestsprints to(stdout on the CLI) instead of
System.err, which is what the redirection was written todo, and
-compactOutputsuppresses it as intended.Tests
RunTestsOutputRedirectTestscompiles a package with one passing and one failing test thatboth print, and asserts the printed output arrives in
RunTests' sink, that-compactOutputsuppresses it, and that the summary and assertion message are unaffected.interpreterOutputIsRedirectedfails on unmodified master and passes with this change;the full
./gradlew testsuite passes.