Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prevent stray output from reaching stdout #618

Merged
merged 6 commits into from
Apr 28, 2024
Merged

Prevent stray output from reaching stdout #618

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5e25435
assert no stray output in coroutines test 7
phiSgr Mar 22, 2023
e23f3cf
do not switch System.out back
phiSgr Mar 18, 2023
b9fc7ca
synchornize over BAOS
phiSgr Mar 22, 2023
d2a0392
stop handling stray output
phiSgr Mar 22, 2023
817bed4
Merge branch 'master' into remove-stray-output
phiSgr Feb 2, 2024
cdf01fe
Merge branch 'master' into remove-stray-output
nikpachoo Apr 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions executors/src/main/kotlin/JUnitExecutors.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,12 @@ class JUnitExecutors {
}
groupedTestResults[testRunInfo.className]?.add(testRunInfo)
nikpachoo marked this conversation as resolved.
Show resolved Hide resolved
}
print(mapper.writeValueAsString(groupedTestResults))
standardOutput.print(mapper.writeValueAsString(groupedTestResults))
}
catch (e: Exception) {
System.setOut(standardOutput)
print("[\"")
standardOutput.print("[\"")
e.printStackTrace()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that it's writing to PrintStream(errorStream). Then the BAOS is discarded. Should it be sent to stderr?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, hard to say. The code in the catch block seems nonfunctional, regardless of the changes. I tried to reproduce a case when the execution flow reached this part of the code and got a serialization error.

print("\"]")
standardOutput.print("\"]")
}
}

Expand Down
8 changes: 3 additions & 5 deletions executors/src/main/kotlin/JavaRunnerExecutor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,13 @@ class JavaRunnerExecutor {
}
System.out.flush()
System.err.flush()
System.setOut(defaultOutputStream)
outputObj.text = outputStream.toString()
outputObj.text = synchronized(outputStream) { outputStream.toString() }
.replace("</errStream><errStream>".toRegex(), "")
.replace("</outStream><outStream>".toRegex(), "")
print(mapper.writeValueAsString(outputObj))
defaultOutputStream.print(mapper.writeValueAsString(outputObj))
}
catch (e: Throwable) {
System.setOut(defaultOutputStream)
System.out.println(mapper.writeValueAsString(RunOutput(exception = e)))
defaultOutputStream.println(mapper.writeValueAsString(RunOutput(exception = e)))
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions executors/src/main/kotlin/OutputStreams.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,21 @@ import kotlin.jvm.Throws
internal class ErrorStream(private val outputStream: OutputStream) : OutputStream() {

@Throws(IOException::class)
override fun write(b: Int) {
override fun write(b: Int) = synchronized(outputStream) {
outputStream.write("<errStream>".toByteArray())
outputStream.write(b)
outputStream.write("</errStream>".toByteArray())
}

@Throws(IOException::class)
override fun write(b: ByteArray) {
override fun write(b: ByteArray) = synchronized(outputStream) {
outputStream.write("<errStream>".toByteArray())
outputStream.write(b)
outputStream.write("</errStream>".toByteArray())
}

@Throws(IOException::class)
override fun write(b: ByteArray, offset: Int, length: Int) {
override fun write(b: ByteArray, offset: Int, length: Int) = synchronized(outputStream) {
outputStream.write("<errStream>".toByteArray())
outputStream.write(b, offset, length)
outputStream.write("</errStream>".toByteArray())
Expand All @@ -32,21 +32,21 @@ internal class ErrorStream(private val outputStream: OutputStream) : OutputStrea
internal class OutStream(private val outputStream: OutputStream) : OutputStream() {

@Throws(IOException::class)
override fun write(b: Int) {
override fun write(b: Int) = synchronized(outputStream) {
outputStream.write("<outStream>".toByteArray())
outputStream.write(b)
outputStream.write("</outStream>".toByteArray())
}

@Throws(IOException::class)
override fun write(b: ByteArray) {
override fun write(b: ByteArray) = synchronized(outputStream) {
outputStream.write("<outStream>".toByteArray())
outputStream.write(b)
outputStream.write("</outStream>".toByteArray())
}

@Throws(IOException::class)
override fun write(b: ByteArray, offset: Int, length: Int) {
override fun write(b: ByteArray, offset: Int, length: Int) = synchronized(outputStream) {
outputStream.write("<outStream>".toByteArray())
outputStream.write(b, offset, length)
outputStream.write("</outStream>".toByteArray())
Expand Down
4 changes: 3 additions & 1 deletion executors/src/main/kotlin/TestListener.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ internal class TestListener : RunListener() {
System.err.flush()
JUnitExecutors.output[JUnitExecutors.output.size - 1].apply {
executionTime = System.currentTimeMillis() - startTime
output = testOutputStream?.toString().orEmpty()
output = testOutputStream?.let {
synchronized(it) { it.toString() }
}.orEmpty()
.replace("</errStream><errStream>".toRegex(), "")
.replace("</outStream><outStream>".toRegex(), "")
}
Expand Down
9 changes: 1 addition & 8 deletions src/main/kotlin/com/compiler/server/model/ProgramOutput.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,7 @@ data class ProgramOutput(
standardOutput.isBlank() -> ExecutionResult()
else -> {
try {
// coroutines can produced incorrect output. see example in `base coroutines test 7`
if (standardOutput.startsWith("{")) outputMapper.readValue(standardOutput, ExecutionResult::class.java)
else {
val result = outputMapper.readValue("{" + standardOutput.substringAfter("{"), ExecutionResult::class.java)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we won't need this workaround, but I wouldn't object to keeping it.

result.apply {
text = standardOutput.substringBefore("{") + text
}
}
outputMapper.readValue(standardOutput, ExecutionResult::class.java)
} catch (e: Exception) {
ExecutionResult(exception = e.toExceptionDescriptor())
}
Expand Down
5 changes: 3 additions & 2 deletions src/test/kotlin/com/compiler/server/CoroutinesRunnerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,11 @@ class CoroutinesRunnerTest : BaseExecutorTest() {

@Test
fun `base coroutines test 7`() {
run(
val result = run(
code = "import kotlinx.coroutines.*\n\nfun main() = runBlocking {\nGlobalScope.launch {\n repeat(1000) { i ->\n println(\"I'm sleeping \$i ...\")\n delay(500L)\n }\n}\ndelay(1300L) // just quit after delay \n}",
contains = "I'm sleeping 0 ...\nI'm sleeping 1 ...\nI'm sleeping 2 ...\n"
contains = ""
)
Assertions.assertEquals("<outStream>I'm sleeping 0 ...\nI'm sleeping 1 ...\nI'm sleeping 2 ...\n</outStream>", result.text)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you go before the fix with git checkout 5e25435, this test will intermittently fail.

}

@Test
Expand Down