From 44555b7cd8e3cc4146e813edc3f9a4ee4546f66f Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 29 Sep 2020 10:52:36 -0700 Subject: [PATCH] Fixes to for escaping of output --- .../jupyter/kernels/cellExecution.ts | 16 +------ .../notebook/executionService.ds.test.ts | 48 +++++++++++++++++++ .../notebook/interrupRestart.ds.test.ts | 4 +- 3 files changed, 52 insertions(+), 16 deletions(-) diff --git a/src/client/datascience/jupyter/kernels/cellExecution.ts b/src/client/datascience/jupyter/kernels/cellExecution.ts index fd75fd8a5096c..8bbedad05b963 100644 --- a/src/client/datascience/jupyter/kernels/cellExecution.ts +++ b/src/client/datascience/jupyter/kernels/cellExecution.ts @@ -38,8 +38,6 @@ import { import { IKernel } from './types'; // tslint:disable-next-line: no-var-requires no-require-imports const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed'); -// tslint:disable-next-line: no-require-imports -import escape = require('lodash/escape'); export class CellExecutionFactory { constructor( @@ -471,11 +469,6 @@ export class CellExecution { // See this for docs on the messages: // https://jupyter-client.readthedocs.io/en/latest/messaging.html#messaging-in-jupyter private async handleExecuteResult(msg: KernelMessage.IExecuteResultMsg, clearState: RefBool) { - // Escape text output - if (msg.content.data && msg.content.data.hasOwnProperty('text/plain')) { - msg.content.data['text/plain'] = escape(msg.content.data['text/plain'] as string); - } - await this.addToCellData( { output_type: 'execute_result', @@ -500,7 +493,7 @@ export class CellExecution { // Mark as stream output so the text is formatted because it likely has ansi codes in it. output_type: 'stream', // tslint:disable-next-line: no-any - text: escape((o.data as any)['text/plain'].toString()), + text: (o.data as any)['text/plain'].toString(), metadata: {}, execution_count: reply.execution_count }, @@ -540,7 +533,7 @@ export class CellExecution { existing.text = formatStreamText(concatMultilineString(existing.text + escape(msg.content.text))); edit.replaceCellOutput(this.cellIndex, [...exitingCellOutput]); // This is necessary to get VS code to update (for now) } else { - const originalText = formatStreamText(concatMultilineString(escape(msg.content.text))); + const originalText = formatStreamText(concatMultilineString(msg.content.text)); // Create a new stream entry const output: nbformat.IStream = { output_type: 'stream', @@ -553,11 +546,6 @@ export class CellExecution { } private async handleDisplayData(msg: KernelMessage.IDisplayDataMsg, clearState: RefBool) { - // Escape text output - if (msg.content.data && msg.content.data.hasOwnProperty('text/plain')) { - msg.content.data['text/plain'] = escape(msg.content.data['text/plain'] as string); - } - const output: nbformat.IDisplayData = { output_type: 'display_data', data: msg.content.data, diff --git a/src/test/datascience/notebook/executionService.ds.test.ts b/src/test/datascience/notebook/executionService.ds.test.ts index ff7d2c882b49e..d66b67701d05b 100644 --- a/src/test/datascience/notebook/executionService.ds.test.ts +++ b/src/test/datascience/notebook/executionService.ds.test.ts @@ -264,4 +264,52 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { // Verify that it hasn't got added (even after interrupting). assertNotHasTextOutputInVSCode(cell, 'Start', 0, false); }); + test('Verify escaping of output', async () => { + await insertPythonCellAndWait('1'); + await insertPythonCellAndWait(dedent` + a="" + a`); + await insertPythonCellAndWait(dedent` + a="" + print(a)`); + await insertPythonCellAndWait('raise Exception("")'); + const cells = vscodeNotebook.activeNotebookEditor?.document.cells!; + + await executeActiveDocument(); + + // Wait till execution count changes and status is error. + await waitForCondition( + async () => assertHasExecutionCompletedWithErrors(cells[3]), + 15_000, + 'Cell did not get executed' + ); + + for (const cell of cells) { + assert.lengthOf(cell.outputs, 1, 'Incorrect output'); + } + assert.equal( + cells[0].outputs[0].outputKind, + vscodeNotebookEnums.CellOutputKind.Rich, + 'Incorrect output for first cell' + ); + assert.equal( + cells[1].outputs[0].outputKind, + vscodeNotebookEnums.CellOutputKind.Rich, + 'Incorrect output for first cell' + ); + assert.equal( + cells[2].outputs[0].outputKind, + vscodeNotebookEnums.CellOutputKind.Rich, + 'Incorrect output for first cell' + ); + assertHasTextOutputInVSCode(cells[0], '1'); + assertHasTextOutputInVSCode(cells[1], '', 0, false); + assertHasTextOutputInVSCode(cells[2], '', 0, false); + const errorOutput = cells[3].outputs[0] as CellErrorOutput; + assert.equal(errorOutput.outputKind, vscodeNotebookEnums.CellOutputKind.Error, 'Incorrect output'); + assert.equal(errorOutput.ename, 'Exception', 'Incorrect ename'); // As status contains ename, we don't want this displayed again. + assert.equal(errorOutput.evalue, '', 'Incorrect evalue'); // As status contains ename, we don't want this displayed again. + assert.isNotEmpty(errorOutput.traceback, 'Incorrect traceback'); + assert.include(errorOutput.traceback.join(''), ''); + }); }); diff --git a/src/test/datascience/notebook/interrupRestart.ds.test.ts b/src/test/datascience/notebook/interrupRestart.ds.test.ts index 7deae6e8bfd50..cd4a12c0e863b 100644 --- a/src/test/datascience/notebook/interrupRestart.ds.test.ts +++ b/src/test/datascience/notebook/interrupRestart.ds.test.ts @@ -81,7 +81,7 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors (slow)', await closeNotebooksAndCleanUpAfterTests(disposables.concat(suiteDisposables)); }); - test('Cancelling token will cancel cell executionxxx', async () => { + test('Cancelling token will cancel cell execution', async () => { await insertPythonCellAndWait('import time\nfor i in range(10000):\n print(i)\n time.sleep(0.1)', 0); const cell = vscEditor.document.cells[0]; const appShell = api.serviceContainer.get(IApplicationShell); @@ -115,7 +115,7 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors (slow)', assertVSCCellHasErrors(cell); } }); - test('Restarting kernel will cancel cell execution & we can re-run a cellxxx', async () => { + test('Restarting kernel will cancel cell execution & we can re-run a cell', async () => { await insertPythonCellAndWait('import time\nfor i in range(10000):\n print(i)\n time.sleep(0.1)', 0); const cell = vscEditor.document.cells[0];