switch to binary mode w/ line buffered #118

index.test.ts

@@ -7,6 +7,7 @@
 import * as assert from 'assert'
 
 import {PythonEvaluator} from './index'
+import { EOL } from 'os';
 
 function isEmpty(obj) {
     return Object.keys(obj).length === 0;
@@ -55,7 +56,7 @@ suite("python_evaluator Tests", () => {
 
     test("arepl_store works", function(done){
         pyEvaluator.onPrint = (result)=>{
-            assert.strictEqual(result, "3")
+            assert.strictEqual(result, "3"+EOL)
         }
 
         input.evalCode = "arepl_store=3"
@@ -141,7 +142,7 @@ suite("python_evaluator Tests", () => {
     test("can print stdout", function(done){
         let hasPrinted = false
         pyEvaluator.onPrint = (stdout)=>{ 
-            assert.equal(stdout, "hello world")
+            assert.equal(stdout, "hello world"+EOL)
             hasPrinted = true
         }
 
@@ -174,38 +175,35 @@ suite("python_evaluator Tests", () => {
     test("can print stderr", function(done){
         let hasLogged = false
         pyEvaluator.onStderr = (stderr)=>{ 
-            assert.equal(stderr, "hello world\r")
+            assert.equal(stderr, "hello world")
             // I have nooo clue why the \r is at the end
             // for some reason python-shell recieves hello world\r\r\n
             hasLogged = true
+            done()
         }
 
         pyEvaluator.onResult = (result) => {
-            if(!hasLogged) assert.fail("program has returned result","program should still be logging")
-            else done()
+            setTimeout(() => {
+                if(!hasLogged) assert.fail("program has returned result "+JSON.stringify(result),"program should still be logging")
+            }, 100); //to avoid race conditions wait a bit in case stderr arrives later
         }
 
-        input.evalCode = "import sys;sys.stderr.write('hello world\\r\\n')"
+        input.evalCode = "import sys;sys.stderr.write('hello world')"
         pyEvaluator.execCode(input)
     })
 
     test("can print multiple lines", function(done){
         let firstPrint = false
-        let secondPrint = false
 
-        pyEvaluator.onPrint = (stdout)=>{ 
-            if(firstPrint){
-                assert.equal(stdout, '2')
-                secondPrint = true
-            }
-            else{
-                assert.equal(stdout, "1")
-                firstPrint = true
-            }
+        pyEvaluator.onPrint = (stdout)=>{
+            // not sure why it is doing this.. stdout should be line buffered
+            // so we should get 1 and 2 seperately
+            assert.equal(stdout, '1'+EOL+'2'+EOL)
+            firstPrint = true
         }
 
         pyEvaluator.onResult = () => {
-            if(!secondPrint) assert.fail("program has returned result","program should still be printing")
+            if(!firstPrint) assert.fail("program has returned result","program should still be printing")
             else done()
         }
 
@@ -215,7 +213,7 @@ suite("python_evaluator Tests", () => {
 
     test("returns result after print", function(done){
         pyEvaluator.onPrint = (stdout)=>{ 
-            assert.equal(stdout, "hello world")
+            assert.equal(stdout, "hello world"+EOL)
             assert.equal(pyEvaluator.executing, true)
         }
 

index.ts

@@ -1,4 +1,5 @@
 import {PythonShell, Options} from 'python-shell' 
+import { EOL } from 'os'
 
 export interface FrameSummary {
 	_line: string
@@ -53,8 +54,8 @@ export interface PythonResult{
 }
 
 export class PythonEvaluator{
-    
-    private static readonly identifier = "6q3co7"
+	
+	private static readonly identifier = "6q3co7"
 	private static readonly areplPythonBackendFolderPath = __dirname + '/python/'
 
     /**
@@ -86,9 +87,10 @@ export class PythonEvaluator{
 			process.env.PATH = ["/usr/local/bin", process.env.PATH].join(":")
 		}
 
-		// we want unbuffered mode by default because it can be frustrating to the user
-		// if they run the program but don't see any print output immediately.
-		if(!options.pythonOptions) this.options.pythonOptions = ['-u']
+		// python-shell buffers untill newline is reached in text mode
+		// so we use binary instead to skip python-shell buffering
+		// this lets user flush without newline
+		this.options.mode = 'binary'
 		if(!options.pythonPath) this.options.pythonPath = PythonShell.defaultPythonPath
 		if(!options.scriptPath) this.options.scriptPath = PythonEvaluator.areplPythonBackendFolderPath
 	}
@@ -101,7 +103,7 @@ export class PythonEvaluator{
 		if(this.executing) return
 		this.executing = true
 		this.startTime = Date.now()
-		this.pyshell.send(JSON.stringify(code))
+		this.pyshell.send(JSON.stringify(code)+EOL)
 	}
 
 	/**
@@ -161,11 +163,11 @@ export class PythonEvaluator{
 	start(){
 		console.log("Starting Python...")
 		this.pyshell = new PythonShell('arepl_python_evaluator.py', this.options)
-		this.pyshell.on('message', message => {
+		this.pyshell.stdout.on('data', message => {
 			this.handleResult(message)
 		})
-		this.pyshell.on('stderr', (log)=>{
-			this.onStderr(log)
+		this.pyshell.stderr.on('data', (log: Buffer)=>{
+			this.onStderr(log.toString())
 		})
 		this.running = true
 	}
@@ -194,7 +196,8 @@ export class PythonEvaluator{
 	 * handles pyshell results and calls onResult / onPrint
 	 * @param {string} results 
 	 */
-	handleResult(results:string) {
+	handleResult(resultsBuffer: Buffer) {
+		let results = resultsBuffer.toString()
 		let pyResult:PythonResult = {
 			userError:null,
 			userErrorMsg: "",
@@ -236,8 +239,8 @@ export class PythonEvaluator{
 				throw err
 			}
 		}
-        else{
-            this.onPrint(results)
+		else{
+			this.onPrint(results)
 		}
 	}
 

python/arepl_python_evaluator.py

@@ -6,7 +6,9 @@
 import traceback
 from time import time
 import asyncio
+from io import TextIOWrapper
 import os
+import sys
 from sys import path, modules, argv, version_info, exc_info
 from typing import Any, Dict, FrozenSet, Set
 from contextlib import contextmanager
@@ -247,5 +249,9 @@ def main(json_input: str):
 
 
 if __name__ == "__main__":
+    # arepl is ran via node so python thinks stdout is not a tty device and uses full buffering
+    # We want users to see output in real time so we change to line buffering
+    # todo: once python3.7 is supported use .reconfigure() instead
+    sys.stdout = TextIOWrapper(open(sys.stdout.fileno(), 'wb'), line_buffering=True)
     while True:
         main(input())