Fix Kotlin#326 - proper handling of store_history

strangepleasures · Sep 9, 2021 · da2870e · da2870e
1 parent 948e92e
commit da2870e
Show file tree

Hide file tree

Showing 8 changed files with 26 additions and 12 deletions.
diff --git a/...lib/test-kit/src/main/kotlin/org/jetbrains/kotlinx/jupyter/testkit/JupyterReplTestCase.kt b/...lib/test-kit/src/main/kotlin/org/jetbrains/kotlinx/jupyter/testkit/JupyterReplTestCase.kt
@@ -16,7 +16,7 @@ abstract class JupyterReplTestCase {
     }
 
     fun execEx(code: Code): EvalResultEx {
-        return repl.evalEx(code, null, -1)
+        return repl.evalEx(code, null, -1, storeHistory)
     }
 
     fun exec(code: Code): Any? {

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(1)
+        val executionCount = AtomicLong(0)
 
         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,9 @@ fun JupyterConnection.Socket.shellMessagesHandler(msg: Message, repl: ReplForJup
 
         is ExecuteRequest -> {
             connection.contextMessage = msg
-            val count = executionCount.getAndIncrement()
+            val count = executionCount.updateAndGet {
+                if (content.storeHistory) it + 1 else it
+            }
             val startedTime = ISO8601DateNow
 
             val displayHandler = SocketDisplayHandler(connection.iopub, repl.notebook, msg)
@@ -284,7 +286,7 @@ fun JupyterConnection.Socket.shellMessagesHandler(msg: Message, repl: ReplForJup
                 runCommand(code, repl)
             } else {
                 connection.evalWithIO(repl, msg) {
-                    repl.eval(code, displayHandler, count.toInt())
+                    repl.eval(code, displayHandler, count.toInt(), content.storeHistory)
                 }
             }
 

diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt
@@ -107,7 +107,7 @@ interface ReplOptions {
 
 interface ReplForJupyter {
 
-    fun eval(code: Code, displayHandler: DisplayHandler? = null, jupyterId: Int = -1): EvalResult
+    fun eval(code: Code, displayHandler: DisplayHandler? = null, jupyterId: Int = -1, storeHistory: Boolean = true): EvalResult
 
     fun <T> eval(execution: ExecutionCallback<T>): T
 
@@ -379,7 +379,7 @@ class ReplForJupyterImpl(
         })
     }
 
-    fun evalEx(code: Code, displayHandler: DisplayHandler?, jupyterId: Int): EvalResultEx {
+    fun evalEx(code: Code, displayHandler: DisplayHandler?, jupyterId: Int, storeHistory: Boolean): EvalResultEx {
         return withEvalContext {
             rethrowAsLibraryException(LibraryProblemPart.BEFORE_CELL_CALLBACKS) {
                 beforeCellExecution.forEach { executor.execute(it) }
@@ -392,7 +392,9 @@ class ReplForJupyterImpl(
             val result = try {
                 log.debug("Current cell id: $jupyterId")
                 executor.execute(code, displayHandler, currentCellId = jupyterId - 1) { internalId, codeToExecute ->
-                    cell = notebook.addCell(internalId, codeToExecute, EvalData(jupyterId, code))
+                    if (storeHistory) {
+                        cell = notebook.addCell(internalId, codeToExecute, EvalData(jupyterId, code))
+                    }
                 }
             } finally {
                 compiledData = internalEvaluator.popAddedCompiledScripts()
@@ -430,8 +432,8 @@ class ReplForJupyterImpl(
         }
     }
 
-    override fun eval(code: Code, displayHandler: DisplayHandler?, jupyterId: Int): EvalResult {
-        return evalEx(code, displayHandler, jupyterId).run { EvalResult(renderedValue, metadata) }
+    override fun eval(code: Code, displayHandler: DisplayHandler?, jupyterId: Int, storeHistory: Boolean): EvalResult {
+        return evalEx(code, displayHandler, jupyterId, storeHistory).run { EvalResult(renderedValue, metadata) }
     }
 
     override fun <T> eval(execution: ExecutionCallback<T>): T {

diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/executeTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/executeTests.kt
@@ -85,12 +85,13 @@ class ExecuteTests : KernelServerTestsBase() {
         executeReplyChecker: (Message) -> Unit = {},
         inputs: List<String> = emptyList(),
         allowStdin: Boolean = true,
+        storeHistory: Boolean = true
     ): Any? {
         try {
             val shell = this.shell!!
             val ioPub = this.ioPub!!
             val stdin = this.stdin!!
-            shell.sendMessage(MessageType.EXECUTE_REQUEST, content = ExecuteRequest(code, allowStdin = allowStdin))
+            shell.sendMessage(MessageType.EXECUTE_REQUEST, content = ExecuteRequest(code, allowStdin = allowStdin, storeHistory = storeHistory))
             inputs.forEach {
                 stdin.sendMessage(MessageType.INPUT_REPLY, InputReply(it))
             }
@@ -312,9 +313,11 @@ class ExecuteTests : KernelServerTestsBase() {
         }
         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) })
 
         assertEquals(jsonObject("text/plain" to "42"), res1)
         assertEquals(jsonObject("text/plain" to "43"), res2)
+        assertEquals(jsonObject("text/plain" to "44"), res3)
     }
 
     @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
@@ -8,5 +8,5 @@ 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)
+        repl.eval(code, displayHandler, jupyterId, jupyterId > 0)
 }
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
@@ -385,6 +385,12 @@ class ReplTests : AbstractSingleReplTest() {
         assertFails { eval("Out[3]") }
     }
 
+    @Test
+    fun testNoHistory() {
+        eval("1+1", null)
+        assertFails { eval("Out[1]") }
+    }
+
     @Test
     fun testOutputMagic() {
         eval("%output --max-cell-size=100500 --no-stdout")

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,7 +48,8 @@ class ReplWithTestResolverTests : AbstractSingleReplTest() {
                 "Bill", 135,
                 "Mark", 160
             ).typed<Unit>()
-            """.trimIndent()
+            """.trimIndent(),
+            jupyterId = 1
         )
 
         val value = res.resultValue