Fix increment logic, introduce structure for the eval() method

ileasile · ileasile · commit ec6f9470ef8b · 2021-09-13T15:05:58.000+03:00
diff --git a/jupyter-lib/test-kit/src/main/kotlin/org/jetbrains/kotlinx/jupyter/testkit/JupyterReplTestCase.kt b/jupyter-lib/test-kit/src/main/kotlin/org/jetbrains/kotlinx/jupyter/testkit/JupyterReplTestCase.kt
@@ -3,6 +3,7 @@ package org.jetbrains.kotlinx.jupyter.testkit
 import io.kotest.matchers.nulls.shouldNotBeNull
 import io.kotest.matchers.types.shouldBeInstanceOf
 import jupyter.kotlin.DependsOn
+import org.jetbrains.kotlinx.jupyter.EvalRequestData
 import org.jetbrains.kotlinx.jupyter.ReplForJupyterImpl
 import org.jetbrains.kotlinx.jupyter.api.Code
 import org.jetbrains.kotlinx.jupyter.api.MimeTypedResult
@@ -16,7 +17,7 @@ abstract class JupyterReplTestCase {
     }
 
     fun execEx(code: Code): EvalResultEx {
-        return repl.evalEx(code, null, -1, true)
+        return repl.evalEx(EvalRequestData(code))
     }
 
     fun exec(code: Code): Any? {
diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt
@@ -95,7 +95,9 @@ class CodeCellImpl(
 class EvalData(
     val executionCounter: Int,
     val rawCode: String,
-)
+) {
+    constructor(evalRequestData: EvalRequestData) : this(evalRequestData.jupyterId, evalRequestData.code)
+}
 
 class NotebookImpl(
     private val runtimeProperties: ReplRuntimeProperties,
diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/ikotlin.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/ikotlin.kt
@@ -108,7 +108,7 @@ fun kernelServer(config: KernelConfig, runtimeProperties: ReplRuntimeProperties
 
         log.info("Begin listening for events")
 
-        val executionCount = AtomicLong(0)
+        val executionCount = AtomicLong(1)
 
         val repl = ReplForJupyterImpl(config, runtimeProperties, scriptReceivers)
 
diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/protocol.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/protocol.kt
@@ -265,7 +265,7 @@ fun JupyterConnection.Socket.shellMessagesHandler(msg: Message, repl: ReplForJup
 
         is ExecuteRequest -> {
             connection.contextMessage = msg
-            val count = executionCount.updateAndGet {
+            val count = executionCount.getAndUpdate {
                 if (content.storeHistory) it + 1 else it
             }
             val startedTime = ISO8601DateNow
@@ -286,7 +286,15 @@ fun JupyterConnection.Socket.shellMessagesHandler(msg: Message, repl: ReplForJup
                 runCommand(code, repl)
             } else {
                 connection.evalWithIO(repl, msg) {
-                    repl.eval(code, displayHandler, count.toInt(), content.storeHistory)
+                    repl.eval(
+                        EvalRequestData(
+                            code,
+                            displayHandler,
+                            count.toInt(),
+                            content.storeHistory,
+                            content.silent,
+                        )
+                    )
                 }
             }
 
diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt
@@ -79,6 +79,14 @@ import kotlin.script.experimental.jvm.jvm
 
 data class CheckResult(val isComplete: Boolean = true)
 
+class EvalRequestData(
+    val code: Code,
+    val displayHandler: DisplayHandler? = null,
+    val jupyterId: Int = -1,
+    val storeHistory: Boolean = true,
+    val isSilent: Boolean = false,
+)
+
 class ReplEvalRuntimeException(message: String, cause: Throwable? = null) : ReplException(message, cause)
 
 enum class ExecutedCodeLogging {
@@ -107,7 +115,8 @@ interface ReplOptions {
 
 interface ReplForJupyter {
 
-    fun eval(code: Code, displayHandler: DisplayHandler? = null, jupyterId: Int = -1, storeHistory: Boolean = true): EvalResult
+    fun eval(code: Code): EvalResult = eval(EvalRequestData(code))
+    fun eval(evalData: EvalRequestData): EvalResult
 
     fun <T> eval(execution: ExecutionCallback<T>): T
 
@@ -379,7 +388,7 @@ class ReplForJupyterImpl(
         })
     }
 
-    fun evalEx(code: Code, displayHandler: DisplayHandler?, jupyterId: Int, storeHistory: Boolean): EvalResultEx {
+    fun evalEx(evalData: EvalRequestData): EvalResultEx {
         return withEvalContext {
             rethrowAsLibraryException(LibraryProblemPart.BEFORE_CELL_CALLBACKS) {
                 beforeCellExecution.forEach { executor.execute(it) }
@@ -390,10 +399,10 @@ class ReplForJupyterImpl(
             val compiledData: SerializedCompiledScriptsData
             val newImports: List<String>
             val result = try {
-                log.debug("Current cell id: $jupyterId")
-                executor.execute(code, displayHandler, currentCellId = jupyterId - 1) { internalId, codeToExecute ->
-                    if (storeHistory) {
-                        cell = notebook.addCell(internalId, codeToExecute, EvalData(jupyterId, code))
+                log.debug("Current cell id: ${evalData.jupyterId}")
+                executor.execute(evalData.code, evalData.displayHandler, currentCellId = evalData.jupyterId - 1) { internalId, codeToExecute ->
+                    if (evalData.storeHistory) {
+                        cell = notebook.addCell(internalId, codeToExecute, EvalData(evalData))
                     }
                 }
             } finally {
@@ -432,8 +441,8 @@ class ReplForJupyterImpl(
         }
     }
 
-    override fun eval(code: Code, displayHandler: DisplayHandler?, jupyterId: Int, storeHistory: Boolean): EvalResult {
-        return evalEx(code, displayHandler, jupyterId, storeHistory).run { EvalResult(renderedValue, metadata) }
+    override fun eval(evalData: EvalRequestData): EvalResult {
+        return evalEx(evalData).run { EvalResult(renderedValue, metadata) }
     }
 
     override fun <T> eval(execution: ExecutionCallback<T>): T {
diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt
@@ -15,7 +15,7 @@ class ApiTest : AbstractSingleReplTest() {
     override val repl = makeSimpleRepl()
 
     private fun jEval(jupyterId: Int, code: String): EvalResult {
-        return repl.eval(code, jupyterId = jupyterId)
+        return eval(code, jupyterId = jupyterId)
     }
 
     @Test
@@ -46,7 +46,7 @@ class ApiTest : AbstractSingleReplTest() {
 
     @Test
     fun testVarsReportFormat() {
-        val res = eval(
+        eval(
             """
             val x = 1
             val y = "abc"
diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/executeTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/executeTests.kt
@@ -311,13 +311,23 @@ class ExecuteTests : KernelServerTestsBase() {
             val data = message.data.content as ExecuteReply
             assertEquals(expectedCounter, data.executionCount)
         }
-        val res1 = doExecute("42", executeReplyChecker = { checkCounter(it, 1) })
-        val res2 = doExecute("43", executeReplyChecker = { checkCounter(it, 2) })
-        val res3 = doExecute("44", storeHistory = false, executeReplyChecker = { checkCounter(it, 2) })
+        val res1 = doExecute("41", executeReplyChecker = { checkCounter(it, 1) })
+        val res2 = doExecute("42", executeReplyChecker = { checkCounter(it, 2) })
+        val res3 = doExecute(
+            " \"\${Out[1]} \${Out[2]}\" ",
+            storeHistory = false,
+            executeReplyChecker = { checkCounter(it, 3) }
+        )
+        val res4 = doExecute(
+            "try { Out[3] } catch(e: ArrayIndexOutOfBoundsException) { null }",
+            storeHistory = false,
+            executeReplyChecker = { checkCounter(it, 3) }
+        )
 
-        assertEquals(jsonObject("text/plain" to "42"), res1)
-        assertEquals(jsonObject("text/plain" to "43"), res2)
-        assertEquals(jsonObject("text/plain" to "44"), res3)
+        assertEquals(jsonObject("text/plain" to "41"), res1)
+        assertEquals(jsonObject("text/plain" to "42"), res2)
+        assertEquals(jsonObject("text/plain" to "41 42"), res3)
+        assertEquals(jsonObject("text/plain" to "null"), res4)
     }
 
     @Test
diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/AbstractSingleReplTest.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/AbstractSingleReplTest.kt
@@ -1,12 +1,13 @@
 package org.jetbrains.kotlinx.jupyter.test.repl
 
 import org.jetbrains.kotlinx.jupyter.DisplayHandler
+import org.jetbrains.kotlinx.jupyter.EvalRequestData
 import org.jetbrains.kotlinx.jupyter.ReplForJupyter
 import org.jetbrains.kotlinx.jupyter.api.Code
 
 abstract class AbstractSingleReplTest : AbstractReplTest() {
     protected abstract val repl: ReplForJupyter
 
-    protected fun eval(code: Code, displayHandler: DisplayHandler? = null, jupyterId: Int = -1) =
-        repl.eval(code, displayHandler, jupyterId, jupyterId > 0)
+    protected fun eval(code: Code, displayHandler: DisplayHandler? = null, jupyterId: Int = -1, storeHistory: Boolean = true) =
+        repl.eval(EvalRequestData(code, displayHandler, jupyterId, storeHistory))
 }
diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt
@@ -5,6 +5,7 @@ import kotlinx.coroutines.runBlocking
 import kotlinx.serialization.encodeToString
 import kotlinx.serialization.json.Json
 import org.jetbrains.kotlinx.jupyter.OutputConfig
+import org.jetbrains.kotlinx.jupyter.ReplEvalRuntimeException
 import org.jetbrains.kotlinx.jupyter.api.VariableStateImpl
 import org.jetbrains.kotlinx.jupyter.exceptions.ReplCompilerException
 import org.jetbrains.kotlinx.jupyter.generateDiagnostic
@@ -23,6 +24,7 @@ import java.io.File
 import kotlin.script.experimental.api.SourceCode
 import kotlin.test.assertEquals
 import kotlin.test.assertFails
+import kotlin.test.assertFailsWith
 import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
 import kotlin.test.fail
@@ -387,8 +389,8 @@ class ReplTests : AbstractSingleReplTest() {
 
     @Test
     fun testNoHistory() {
-        eval("1+1", null)
-        assertFails { eval("Out[1]") }
+        eval("1+1", storeHistory = false)
+        assertFailsWith<ReplEvalRuntimeException>("There is no cell with number '1'") { eval("Out[1]") }
     }
 
     @Test
@@ -484,7 +486,7 @@ class ReplVarsTest : AbstractSingleReplTest() {
             """.trimIndent()
         )
 
-        val varsUpdate = mutableMapOf<String, String>(
+        val varsUpdate = mutableMapOf(
             "x" to "1",
             "y" to "0",
             "z" to "47"
diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplWithTestResolverTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplWithTestResolverTests.kt
@@ -48,8 +48,7 @@ class ReplWithTestResolverTests : AbstractSingleReplTest() {
                 "Bill", 135,
                 "Mark", 160
             ).typed<Unit>()
-            """.trimIndent(),
-            jupyterId = 1
+            """.trimIndent()
         )
 
         val value = res.resultValue

Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@ package org.jetbrains.kotlinx.jupyter.testkit`
`3`	`3`	`import io.kotest.matchers.nulls.shouldNotBeNull`
`4`	`4`	`import io.kotest.matchers.types.shouldBeInstanceOf`
`5`	`5`	`import jupyter.kotlin.DependsOn`
	`6`	`+import org.jetbrains.kotlinx.jupyter.EvalRequestData`
`6`	`7`	`import org.jetbrains.kotlinx.jupyter.ReplForJupyterImpl`
`7`	`8`	`import org.jetbrains.kotlinx.jupyter.api.Code`
`8`	`9`	`import org.jetbrains.kotlinx.jupyter.api.MimeTypedResult`
`@@ -16,7 +17,7 @@ abstract class JupyterReplTestCase {`
`16`	`17`	`}`
`17`	`18`
`18`	`19`	`fun execEx(code: Code): EvalResultEx {`
`19`		`- return repl.evalEx(code, null, -1, true)`
	`20`	`+ return repl.evalEx(EvalRequestData(code))`
`20`	`21`	`}`
`21`	`22`
`22`	`23`	`fun exec(code: Code): Any? {`
Original file line number	Diff line number	Diff line change
`@@ -48,8 +48,7 @@ class ReplWithTestResolverTests : AbstractSingleReplTest() {`
`48`	`48`	`"Bill", 135,`
`49`	`49`	`"Mark", 160`
`50`	`50`	`).typed<Unit>()`
`51`		`- """.trimIndent(),`
`52`		`- jupyterId = 1`
	`51`	`+ """.trimIndent()`
`53`	`52`	`)`
`54`	`53`
`55`	`54`	`val value = res.resultValue`