From f8211a96cdb965b628d49a6ac796ec922111c2bf Mon Sep 17 00:00:00 2001 From: rchiodo Date: Mon, 28 Sep 2020 14:31:16 -0700 Subject: [PATCH 1/5] Fixes for escaping --- src/client/datascience/jupyter/jupyterDebugger.ts | 4 +++- src/datascience-ui/interactive-common/cellOutput.tsx | 7 ++++++- src/test/datascience/notebook.functional.test.ts | 6 +++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/client/datascience/jupyter/jupyterDebugger.ts b/src/client/datascience/jupyter/jupyterDebugger.ts index afa7df942137..7800aa124439 100644 --- a/src/client/datascience/jupyter/jupyterDebugger.ts +++ b/src/client/datascience/jupyter/jupyterDebugger.ts @@ -3,6 +3,8 @@ 'use strict'; import type { nbformat } from '@jupyterlab/coreutils'; import { inject, injectable, named } from 'inversify'; +// tslint:disable-next-line: no-require-imports +import unescape = require('lodash/unescape'); import * as path from 'path'; import * as uuid from 'uuid/v4'; import { DebugConfiguration, Disposable } from 'vscode'; @@ -474,7 +476,7 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener { const data = outputs[0].data; if (data && data.hasOwnProperty('text/plain')) { // tslint:disable-next-line:no-any - return (data as any)['text/plain']; + return unescape((data as any)['text/plain']); } if (outputs[0].output_type === 'stream') { const stream = outputs[0] as nbformat.IStream; diff --git a/src/datascience-ui/interactive-common/cellOutput.tsx b/src/datascience-ui/interactive-common/cellOutput.tsx index b9a9d5bb2eee..8b545eac7086 100644 --- a/src/datascience-ui/interactive-common/cellOutput.tsx +++ b/src/datascience-ui/interactive-common/cellOutput.tsx @@ -314,7 +314,12 @@ export class CellOutput extends React.Component { input = JSON.stringify(output.data); renderWithScrollbars = true; isText = true; - } else if (output.output_type === 'execute_result' && input && input.hasOwnProperty('text/plain')) { + } else if ( + output.output_type === 'execute_result' && + input && + input.hasOwnProperty('text/plain') && + !input.hasOwnProperty('text/html') + ) { // Plain text should actually be shown as html so that escaped HTML shows up correctly mimeType = 'text/html'; isText = true; diff --git a/src/test/datascience/notebook.functional.test.ts b/src/test/datascience/notebook.functional.test.ts index aeec51594400..b8ff1d720cbd 100644 --- a/src/test/datascience/notebook.functional.test.ts +++ b/src/test/datascience/notebook.functional.test.ts @@ -182,7 +182,7 @@ suite('DataScience notebook tests', () => { const error = cell.outputs[0].evalue; if (error) { assert.ok(error, 'Error not found when expected'); - assert.equal(error, errorString, 'Unexpected error found'); + assert.ok(error.toString().includes(errorString), 'Unexpected error found'); } } @@ -757,7 +757,7 @@ suite('DataScience notebook tests', () => { await server!.waitForIdle(10000); console.log('Verifying restart'); - await verifyError(server, 'a', `name 'a' is not defined`); + await verifyError(server, 'a', `is not defined`); } catch (exc) { assert.ok( exc instanceof JupyterKernelPromiseFailedError, @@ -1031,7 +1031,7 @@ a`, mimeType: 'text/plain', cellType: 'code', result: ``, - verifyValue: (d) => assert.equal(d, escape(``), 'XML not escaped') + verifyValue: (d) => assert.ok(d.includes(escape(``)), 'XML not escaped') }, { markdownRegEx: undefined, From 371ac9e56214213e913b0514ba6649c46edbee50 Mon Sep 17 00:00:00 2001 From: rchiodo Date: Mon, 28 Sep 2020 15:07:45 -0700 Subject: [PATCH 2/5] Push a comment ot start PR again --- src/client/datascience/jupyter/jupyterDebugger.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/client/datascience/jupyter/jupyterDebugger.ts b/src/client/datascience/jupyter/jupyterDebugger.ts index 7800aa124439..796dd18ab771 100644 --- a/src/client/datascience/jupyter/jupyterDebugger.ts +++ b/src/client/datascience/jupyter/jupyterDebugger.ts @@ -475,6 +475,8 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener { if (outputs.length > 0) { const data = outputs[0].data; if (data && data.hasOwnProperty('text/plain')) { + // Plain text should be escaped by our execution engine. Unescape it so + // we can parse it. // tslint:disable-next-line:no-any return unescape((data as any)['text/plain']); } From c6543d62d7d63ee7276aa7bd57813f380e42be4d Mon Sep 17 00:00:00 2001 From: rchiodo Date: Mon, 28 Sep 2020 15:18:23 -0700 Subject: [PATCH 3/5] Cache task is failing --- build/ci/templates/steps/initialization.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/ci/templates/steps/initialization.yml b/build/ci/templates/steps/initialization.yml index ed6c6b0c4fb4..6f6984843a02 100644 --- a/build/ci/templates/steps/initialization.yml +++ b/build/ci/templates/steps/initialization.yml @@ -76,7 +76,7 @@ steps: # See the help here on how to cache npm # https://docs.microsoft.com/en-us/azure/devops/pipelines/caching/?view=azure-devops#nodejsnpm - - task: CacheBeta@0 + - task: Cache@2 inputs: key: npm | $(Agent.OS) | package-lock.json path: $(npm_config_cache) From eca474c8932369dec341061acfe992c46231c5ce Mon Sep 17 00:00:00 2001 From: rchiodo Date: Mon, 28 Sep 2020 16:04:50 -0700 Subject: [PATCH 4/5] Remove cache task --- build/ci/templates/steps/initialization.yml | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/build/ci/templates/steps/initialization.yml b/build/ci/templates/steps/initialization.yml index 6f6984843a02..59844e4d5cde 100644 --- a/build/ci/templates/steps/initialization.yml +++ b/build/ci/templates/steps/initialization.yml @@ -76,13 +76,14 @@ steps: # See the help here on how to cache npm # https://docs.microsoft.com/en-us/azure/devops/pipelines/caching/?view=azure-devops#nodejsnpm - - task: Cache@2 - inputs: - key: npm | $(Agent.OS) | package-lock.json - path: $(npm_config_cache) - restoreKeys: | - npm | $(Agent.OS) - displayName: Cache npm + # Disabled for now until DevOps fixes outage + # - task: Cache@2 + # inputs: + # key: npm | $(Agent.OS) | package-lock.json + # path: $(npm_config_cache) + # restoreKeys: | + # npm | $(Agent.OS) + # displayName: Cache npm - task: Npm@1 displayName: 'npm ci' From 8b06e3cf68d76fe1c0c4549ffeca97fb8e62a406 Mon Sep 17 00:00:00 2001 From: rchiodo Date: Mon, 28 Sep 2020 16:26:25 -0700 Subject: [PATCH 5/5] Not fixing so just put back cache task --- build/ci/templates/steps/initialization.yml | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/build/ci/templates/steps/initialization.yml b/build/ci/templates/steps/initialization.yml index 59844e4d5cde..ed6c6b0c4fb4 100644 --- a/build/ci/templates/steps/initialization.yml +++ b/build/ci/templates/steps/initialization.yml @@ -76,14 +76,13 @@ steps: # See the help here on how to cache npm # https://docs.microsoft.com/en-us/azure/devops/pipelines/caching/?view=azure-devops#nodejsnpm - # Disabled for now until DevOps fixes outage - # - task: Cache@2 - # inputs: - # key: npm | $(Agent.OS) | package-lock.json - # path: $(npm_config_cache) - # restoreKeys: | - # npm | $(Agent.OS) - # displayName: Cache npm + - task: CacheBeta@0 + inputs: + key: npm | $(Agent.OS) | package-lock.json + path: $(npm_config_cache) + restoreKeys: | + npm | $(Agent.OS) + displayName: Cache npm - task: Npm@1 displayName: 'npm ci'