From 6e59c2e079728392fc4be7d277831e5bffdcc1d6 Mon Sep 17 00:00:00 2001 From: Ilya Chernikov Date: Mon, 27 Jun 2022 14:01:46 +0200 Subject: [PATCH] Scripting: fix CLI REPL error reporting behavior discrepancy after switching to the new REPL internals was discovered also on IJ plugin tests, so added a test emulating it. also some test infra refactoring --- compiler/testData/repl/syntaxErrors.repl | 3 + .../jetbrains/kotlin/cli/LauncherReplTest.kt | 60 ++++++++++++++++--- .../repl/AbstractReplInterpreterTest.kt | 24 +++++++- .../compiler/plugin/repl/ReplFromTerminal.kt | 4 +- .../compiler/plugin/repl/ReplInterpreter.kt | 6 +- 5 files changed, 80 insertions(+), 17 deletions(-) diff --git a/compiler/testData/repl/syntaxErrors.repl b/compiler/testData/repl/syntaxErrors.repl index cba37d25db0..507d83d928a 100644 --- a/compiler/testData/repl/syntaxErrors.repl +++ b/compiler/testData/repl/syntaxErrors.repl @@ -2,6 +2,9 @@ error: expecting an element )( ^ +error: expecting an expression +)( + ^ >>> fun foo() = 98 >>> foo() 98 diff --git a/compiler/tests/org/jetbrains/kotlin/cli/LauncherReplTest.kt b/compiler/tests/org/jetbrains/kotlin/cli/LauncherReplTest.kt index c073c3bae6e..a1cf26ebc47 100644 --- a/compiler/tests/org/jetbrains/kotlin/cli/LauncherReplTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/cli/LauncherReplTest.kt @@ -49,12 +49,12 @@ class LauncherReplTest : TestCaseWithTmpdir() { val out = ArrayList() val exceptionContainer = ExceptionContainer() val thread = thread { - val promptRegex = Regex("(?:\\u001B\\p{Graph}+)*(?:>>>|\\.\\.\\.)") + val promptRegex = Regex("(?:\\u001B\\p{Graph}+)*(?:>>> ?|\\.\\.\\.)") try { reader().forEachLine { rawLine -> promptRegex.split(rawLine).forEach { line -> if (line.isNotEmpty()) { - out.add(line.trim()) + out.add(line.trimEnd()) } } } @@ -137,13 +137,24 @@ class LauncherReplTest : TestCaseWithTmpdir() { Assert.fail("missing output for expected patterns:\n${inputsToExpectedOutputsIter.asSequence().joinToString("\n") { it.second } }") } if (!inputsToExpectedOutputsIter.hasNext() || !actualIter.hasNext()) break - val (input, expectedPattern) = inputsToExpectedOutputsIter.next() - val actualLine = actualIter.next() - val strippedActualLine = if (input != null) actualLine.removePrefix(input) else actualLine - assertTrue( - "line \"$strippedActualLine\" do not match with expected pattern \"$expectedPattern\"", - Regex(expectedPattern).matches(strippedActualLine) - ) + var (input, expectedPattern) = inputsToExpectedOutputsIter.next() + var actualLine = actualIter.next() + while (input != null) { + if (actualLine.startsWith(input)) { + actualLine = actualLine.substring(input.length) + } else if (expectedPattern.isEmpty() && actualLine.isNotEmpty() && inputsToExpectedOutputsIter.hasNext()) { + // assuming that on some configs input is not repeated if producing empty output + // in this case trying to check the next expected output + val nextInputToOutput = inputsToExpectedOutputsIter.next() + expectedPattern = nextInputToOutput.second + input = nextInputToOutput.first + continue + } + break + } + if (!Regex(expectedPattern).matches(actualLine)) { + fail("line \"$actualLine\" do not match with expected pattern \"$expectedPattern\"") + } } } @@ -159,6 +170,37 @@ class LauncherReplTest : TestCaseWithTmpdir() { ) } + fun testSReplWithMultipleErrors() { + runInteractive( + *replOutHeader, + "\$;\$;" to ".*expecting an element", + null to "\\\$;\\\$;", + null to "\\^", + null to ".*expecting an element", + null to "\\\$;\\\$;", + null to " \\^", + "println(\$);println(\$);" to ".*expecting an expression", + null to "println\\(\\\$\\);println\\(\\\$\\);", + null to " \\^", + null to ".*expecting '\\)'", + null to "println\\(\\\$\\);println\\(\\\$\\);", + null to " \\^", + null to ".*expecting an element", + null to "println\\(\\\$\\);println\\(\\\$\\);", + null to " \\^", + null to ".*expecting an expression", + null to "println\\(\\\$\\);println\\(\\\$\\);", + null to " \\^", + null to ".*expecting '\\)'", + null to "println\\(\\\$\\);println\\(\\\$\\);", + null to " \\^", + null to ".*expecting an element", + null to "println\\(\\\$\\);println\\(\\\$\\);", + null to " \\^", + "println(42)" to "42", + ) + } + fun testReplResultFormatting() { runInteractive( *replOutHeader, diff --git a/compiler/tests/org/jetbrains/kotlin/repl/AbstractReplInterpreterTest.kt b/compiler/tests/org/jetbrains/kotlin/repl/AbstractReplInterpreterTest.kt index 003c01f0385..3d4caaee3d8 100644 --- a/compiler/tests/org/jetbrains/kotlin/repl/AbstractReplInterpreterTest.kt +++ b/compiler/tests/org/jetbrains/kotlin/repl/AbstractReplInterpreterTest.kt @@ -29,7 +29,9 @@ import org.jetbrains.kotlin.test.KotlinTestUtils import org.jetbrains.kotlin.test.TestJdkKind import org.jetbrains.kotlin.test.testFramework.KtUsefulTestCase import org.junit.Assert +import java.io.ByteArrayOutputStream import java.io.File +import java.io.PrintStream import java.io.PrintWriter import java.util.* import java.util.regex.Pattern @@ -86,6 +88,24 @@ abstract class AbstractReplInterpreterTest : KtUsefulTestCase() { return result } + internal fun captureOutErrRet(body: () -> T): Triple { + val outStream = ByteArrayOutputStream() + val errStream = ByteArrayOutputStream() + val prevOut = System.out + val prevErr = System.err + System.setOut(PrintStream(outStream)) + System.setErr(PrintStream(errStream)) + val ret = try { + body() + } finally { + System.out.flush() + System.err.flush() + System.setOut(prevOut) + System.setErr(prevErr) + } + return Triple(outStream.toString().trim(), errStream.toString().trim(), ret) + } + protected fun doTest(path: String) { val configuration = KotlinTestUtils.newConfiguration(ConfigurationKind.ALL, TestJdkKind.MOCK_JDK) loadScriptingPlugin(configuration) @@ -101,7 +121,7 @@ abstract class AbstractReplInterpreterTest : KtUsefulTestCase() { ) for ((code, expected) in loadLines(File(path))) { - val lineResult = repl.eval(code) + val (output, _, lineResult) = captureOutErrRet { repl.eval(code) } if (DUMP_BYTECODE) { repl.dumpClasses(PrintWriter(System.out)) @@ -109,7 +129,7 @@ abstract class AbstractReplInterpreterTest : KtUsefulTestCase() { val actual = when (lineResult) { is ReplEvalResult.ValueResult -> lineResult.value.toString() - is ReplEvalResult.Error.CompileTime -> MessageRenderer.WITHOUT_PATHS.render(CompilerMessageSeverity.ERROR, lineResult.message, lineResult.location) + is ReplEvalResult.Error.CompileTime -> output is ReplEvalResult.Error -> lineResult.message is ReplEvalResult.Incomplete -> INCOMPLETE_LINE_MESSAGE is ReplEvalResult.UnitResult -> "" diff --git a/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/repl/ReplFromTerminal.kt b/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/repl/ReplFromTerminal.kt index 9cd7a96446d..f7e0fd41692 100644 --- a/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/repl/ReplFromTerminal.kt +++ b/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/repl/ReplFromTerminal.kt @@ -139,8 +139,8 @@ class ReplFromTerminal( writer.outputCommandResult(tryInterpretResultAsValueClass(evalResult) ?: evalResult.toString()) } } - is ReplEvalResult.Error.Runtime -> writer.outputRuntimeError(evalResult.message) - is ReplEvalResult.Error.CompileTime -> writer.outputCompileError(evalResult.message) + is ReplEvalResult.Error.Runtime -> if (evalResult.message.isNotEmpty()) writer.outputRuntimeError(evalResult.message) + is ReplEvalResult.Error.CompileTime -> if (evalResult.message.isNotEmpty()) writer.outputCompileError(evalResult.message) is ReplEvalResult.Incomplete -> writer.notifyIncomplete() is ReplEvalResult.HistoryMismatch -> {} // assuming handled elsewhere } diff --git a/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/repl/ReplInterpreter.kt b/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/repl/ReplInterpreter.kt index 33c46b02bba..eabe5a8e770 100644 --- a/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/repl/ReplInterpreter.kt +++ b/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/repl/ReplInterpreter.kt @@ -174,12 +174,11 @@ class ReplInterpreter( when (val compileResult = compiler.compile(listOf(snippet), compilationConfiguration)) { is ResultWithDiagnostics.Failure -> { val incompleteReport = compileResult.reports.find { it.code == ScriptDiagnostic.incompleteCode } - val firstError = compileResult.reports.find { it.isError() } if (incompleteReport != null) ReplEvalResult.Incomplete(incompleteReport.message) else { compileResult.reportToMessageCollector() - ReplEvalResult.Error.CompileTime(firstError?.message ?: "", firstError?.location?.toCompilerMessageLocation()) + ReplEvalResult.Error.CompileTime("") } } is ResultWithDiagnostics.Success -> { @@ -195,9 +194,8 @@ class ReplInterpreter( } } else -> { - val firstError = evalResult.reports.find { it.isError() } evalResult.reportToMessageCollector() - ReplEvalResult.Error.Runtime(firstError?.message ?: "", firstError?.exception) + ReplEvalResult.Error.Runtime("") } } }