From f88b5faacab5a0a29fb77d927db6011d431cfa26 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 11 Mar 2022 08:08:50 +1100 Subject: [PATCH 01/10] Fix tests --- .../notebook/executionService.vscode.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/test/datascience/notebook/executionService.vscode.test.ts b/src/test/datascience/notebook/executionService.vscode.test.ts index 46154676dd3c..511f05f8562a 100644 --- a/src/test/datascience/notebook/executionService.vscode.test.ts +++ b/src/test/datascience/notebook/executionService.vscode.test.ts @@ -419,24 +419,26 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { await Promise.all([ runAllCellsInActiveNotebook(), - waitForCellExecutionToComplete(cell3), - waitForCellHavingOutput(cell3) + waitForCellExecutionToComplete(cell1), + waitForCellHavingOutput(cell1) ]); // On windows `!where python`, prints multiple items in the output (all executables found). const shellExecutable = getCellOutputs(cell1).trim().split('\n')[0].trim(); - const sysExecutable = getCellOutputs(cell3).trim(); // Sometimes the IPython can (sometimes) fail with an error of `shell not found`. // For now, we'll ignore these errors // We already have tests that ensures the first path in sys.path points to where the executable is located. // Hence skipping this test in such cases is acceptable. - if (hasErrorOutput(cell3.outputs)) { + if (hasErrorOutput(cell1.outputs)) { const errorOutput = translateCellErrorOutput(cell3.outputs[0]); if (errorOutput.traceback.includes('shell not found') || errorOutput.evalue.includes('shell not found')) { return this.skip(); } } + await Promise.all([waitForCellExecutionToComplete(cell3), waitForCellHavingOutput(cell3)]); + + const sysExecutable = getCellOutputs(cell3).trim(); // First path in PATH must be the directory where executable is located. assert.ok( From e0e5c5d74bbe67ecd6bca29df993172541fea8e1 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 11 Mar 2022 08:54:52 +1100 Subject: [PATCH 02/10] Misc --- src/test/datascience/notebook/executionService.vscode.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/datascience/notebook/executionService.vscode.test.ts b/src/test/datascience/notebook/executionService.vscode.test.ts index 511f05f8562a..8aa4990e8daf 100644 --- a/src/test/datascience/notebook/executionService.vscode.test.ts +++ b/src/test/datascience/notebook/executionService.vscode.test.ts @@ -431,7 +431,7 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { // We already have tests that ensures the first path in sys.path points to where the executable is located. // Hence skipping this test in such cases is acceptable. if (hasErrorOutput(cell1.outputs)) { - const errorOutput = translateCellErrorOutput(cell3.outputs[0]); + const errorOutput = translateCellErrorOutput(cell1.outputs[0]); if (errorOutput.traceback.includes('shell not found') || errorOutput.evalue.includes('shell not found')) { return this.skip(); } From 9a0b25eedac178e5053fe2746ca70b49ed5fc340 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 11 Mar 2022 14:45:45 +1100 Subject: [PATCH 03/10] Misc --- .../notebook/executionService.vscode.test.ts | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/test/datascience/notebook/executionService.vscode.test.ts b/src/test/datascience/notebook/executionService.vscode.test.ts index 8aa4990e8daf..1efda668abed 100644 --- a/src/test/datascience/notebook/executionService.vscode.test.ts +++ b/src/test/datascience/notebook/executionService.vscode.test.ts @@ -420,19 +420,39 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { await Promise.all([ runAllCellsInActiveNotebook(), waitForCellExecutionToComplete(cell1), - waitForCellHavingOutput(cell1) + waitForCellHavingOutput(cell1), + waitForCondition( + async () => { + if (getCellOutputs(cell1).trim().length > 0) { + return true; + } + if (hasErrorOutput(cell1.outputs)) { + return true; + } + return false; + }, + defaultNotebookTestTimeout, + 'Cell did not have output' + ) ]); // On windows `!where python`, prints multiple items in the output (all executables found). - const shellExecutable = getCellOutputs(cell1).trim().split('\n')[0].trim(); + const cell1Output = getCellOutputs(cell1); + const shellExecutable = cell1Output + .trim() + .split('\n') + .filter((item) => item.length)[0] + .trim(); // Sometimes the IPython can (sometimes) fail with an error of `shell not found`. // For now, we'll ignore these errors // We already have tests that ensures the first path in sys.path points to where the executable is located. // Hence skipping this test in such cases is acceptable. + let errorOutput = ''; if (hasErrorOutput(cell1.outputs)) { - const errorOutput = translateCellErrorOutput(cell1.outputs[0]); - if (errorOutput.traceback.includes('shell not found') || errorOutput.evalue.includes('shell not found')) { + const error = translateCellErrorOutput(cell1.outputs[0]); + errorOutput = `${error.evalue}:${error.traceback}`; + if (errorOutput.includes('shell not found')) { return this.skip(); } } @@ -443,7 +463,7 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { // First path in PATH must be the directory where executable is located. assert.ok( areInterpreterPathsSame(shellExecutable.toLowerCase(), sysExecutable.toLowerCase()), - `Python paths do not match ${shellExecutable}, ${sysExecutable}` + `Python paths do not match ${shellExecutable}, ${sysExecutable}. Output is ${cell1Output}, error is ${errorOutput}` ); }); test('Testing streamed output', async () => { From c60a6b1312df74fc1c55f147e457591be6fb2573 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 15 Mar 2022 09:06:16 +1100 Subject: [PATCH 04/10] More logging --- .../datascience/notebook/executionService.vscode.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/datascience/notebook/executionService.vscode.test.ts b/src/test/datascience/notebook/executionService.vscode.test.ts index 1efda668abed..e94b409278ce 100644 --- a/src/test/datascience/notebook/executionService.vscode.test.ts +++ b/src/test/datascience/notebook/executionService.vscode.test.ts @@ -463,7 +463,11 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { // First path in PATH must be the directory where executable is located. assert.ok( areInterpreterPathsSame(shellExecutable.toLowerCase(), sysExecutable.toLowerCase()), - `Python paths do not match ${shellExecutable}, ${sysExecutable}. Output is ${cell1Output}, error is ${errorOutput}` + `Python paths do not match ${shellExecutable}, ${sysExecutable}. Output is (${ + cell1.outputs.length + }) (${cell1.outputs + .map((item) => item.items.map((i) => i.mime).join(',')) + .join('#')})${cell1Output}, error is ${errorOutput}` ); }); test('Testing streamed output', async () => { From 825ee4104011be442ea9162cb3d441488b1833ab Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 15 Mar 2022 10:11:07 +1100 Subject: [PATCH 05/10] Added logging --- src/notebooks/execution/cellExecution.ts | 6 ++++++ .../datascience/notebook/executionService.vscode.test.ts | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/notebooks/execution/cellExecution.ts b/src/notebooks/execution/cellExecution.ts index ae17f59d7f02..ea143b8e110d 100644 --- a/src/notebooks/execution/cellExecution.ts +++ b/src/notebooks/execution/cellExecution.ts @@ -634,6 +634,7 @@ export class CellExecution implements IDisposable { // In such circumstances, create a temporary task & use that to update the output (only cell execution tasks can update cell output). const task = this.execution || this.createTemporaryTask(); this.clearLastUsedStreamOutput(); + traceCellMessage(this.cell, 'Append output in addToCellData'); task?.appendOutput([cellOutput]).then(noop, noop); this.endTemporaryTask(); } @@ -755,6 +756,7 @@ export class CellExecution implements IDisposable { // Clear output if waiting for a clear const clearOutput = clearState.value; if (clearOutput) { + traceCellMessage(this.cell, 'Clear cell output'); this.clearLastUsedStreamOutput(); task?.clearOutput().then(noop, noop); clearState.update(false); @@ -789,6 +791,7 @@ export class CellExecution implements IDisposable { name: msg.content.name, text: this.lastUsedStreamOutput.text }); + traceCellMessage(this.cell, `Replace output items ${this.lastUsedStreamOutput.text.substring(0, 100)}`); task?.replaceOutputItems(output.items, this.lastUsedStreamOutput.output).then(noop, noop); } else if (clearOutput) { // Replace the current outputs with a single new output. @@ -799,6 +802,7 @@ export class CellExecution implements IDisposable { text }); this.lastUsedStreamOutput = { output, stream: msg.content.name, text }; + traceCellMessage(this.cell, `Replace output ${this.lastUsedStreamOutput.text.substring(0, 100)}`); task?.replaceOutput([output]).then(noop, noop); } else { // Create a new output @@ -809,6 +813,7 @@ export class CellExecution implements IDisposable { text }); this.lastUsedStreamOutput = { output, stream: msg.content.name, text }; + traceCellMessage(this.cell, `Append output ${this.lastUsedStreamOutput.text.substring(0, 100)}`); task?.appendOutput([output]).then(noop, noop); } this.endTemporaryTask(); @@ -909,6 +914,7 @@ export class CellExecution implements IDisposable { // This message could have come from a background thread. // In such circumstances, create a temporary task & use that to update the output (only cell execution tasks can update cell output). const task = this.execution || this.createTemporaryTask(); + traceCellMessage(this.cell, `Replace output items in display data ${newOutput.items.length}`); task?.replaceOutputItems(newOutput.items, outputToBeUpdated).then(noop, noop); this.endTemporaryTask(); } diff --git a/src/test/datascience/notebook/executionService.vscode.test.ts b/src/test/datascience/notebook/executionService.vscode.test.ts index e94b409278ce..1aefcba7e95c 100644 --- a/src/test/datascience/notebook/executionService.vscode.test.ts +++ b/src/test/datascience/notebook/executionService.vscode.test.ts @@ -423,7 +423,8 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { waitForCellHavingOutput(cell1), waitForCondition( async () => { - if (getCellOutputs(cell1).trim().length > 0) { + const output = getCellOutputs(cell1).trim(); + if (output !== '' && output.length > 0) { return true; } if (hasErrorOutput(cell1.outputs)) { From 6bf6269dd2cc346ad119cf30fc78612d019c0142 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 16 Mar 2022 05:36:32 +1100 Subject: [PATCH 06/10] Fixes --- src/test/datascience/notebook/helper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/datascience/notebook/helper.ts b/src/test/datascience/notebook/helper.ts index b47c56efb20c..cc4dab90f444 100644 --- a/src/test/datascience/notebook/helper.ts +++ b/src/test/datascience/notebook/helper.ts @@ -560,7 +560,7 @@ export async function waitForCellHavingOutput(cell: NotebookCell) { return waitForCondition( async () => { const cellOutputs = getCellOutputs(cell); - return cellOutputs.length > 0 && !cellOutputs[0].includes('No cell outputs'); + return cellOutputs.length > 0 && !cellOutputs.includes('No cell outputs'); }, defaultNotebookTestTimeout, 'No output' From f223a2707800e8d44c307bebece9365009adb398 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 16 Mar 2022 05:39:31 +1100 Subject: [PATCH 07/10] Misc --- .../jupyter/kernels/installationPrompts.vscode.test.ts | 5 +++-- src/test/datascience/notebook/helper.ts | 7 ------- src/test/datascience/notebook/kernelSysPath.vscode.test.ts | 4 ++-- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/test/datascience/jupyter/kernels/installationPrompts.vscode.test.ts b/src/test/datascience/jupyter/kernels/installationPrompts.vscode.test.ts index 1a7303176b4f..e5b6facc5765 100644 --- a/src/test/datascience/jupyter/kernels/installationPrompts.vscode.test.ts +++ b/src/test/datascience/jupyter/kernels/installationPrompts.vscode.test.ts @@ -49,7 +49,8 @@ import { waitForKernelToChange, insertCodeCell, waitForExecutionCompletedSuccessfully, - getCellOutputs + getCellOutputs, + waitForCellHavingOutput } from '../../notebook/helper'; import * as kernelSelector from '../../../../notebooks/controllers/kernelSelector'; @@ -568,7 +569,7 @@ suite('DataScience Install IPyKernel (slow) (install)', function () { ? Promise.resolve() : waitForIPyKernelToGetInstalled(), waitForExecutionCompletedSuccessfully(cell), - waitForCondition(async () => cell.outputs.length > 0, defaultNotebookTestTimeout, 'No cell output') + waitForCellHavingOutput(cell) ]); console.log('Stepd'); diff --git a/src/test/datascience/notebook/helper.ts b/src/test/datascience/notebook/helper.ts index cc4dab90f444..d6ca658a4cb1 100644 --- a/src/test/datascience/notebook/helper.ts +++ b/src/test/datascience/notebook/helper.ts @@ -753,13 +753,6 @@ export async function waitForTextOutput( }, it is ${getCellOutputs(cell)}` ); } -export async function waitForCellToHaveOutput(cell: NotebookCell, timeout = defaultNotebookTestTimeout) { - await waitForCondition( - async () => cell.outputs.length > 0, - timeout, - () => `No outputs for Cell ${cell.index + 1}` - ); -} export function assertNotHasTextOutputInVSCode(cell: NotebookCell, text: string, index: number, isExactMatch = true) { const cellOutputs = cell.outputs; assert.ok(cellOutputs, 'No output'); diff --git a/src/test/datascience/notebook/kernelSysPath.vscode.test.ts b/src/test/datascience/notebook/kernelSysPath.vscode.test.ts index a2c4f7c69e5d..d8fb480d4773 100644 --- a/src/test/datascience/notebook/kernelSysPath.vscode.test.ts +++ b/src/test/datascience/notebook/kernelSysPath.vscode.test.ts @@ -23,7 +23,7 @@ import { startJupyterServer, waitForExecutionCompletedSuccessfully, waitForKernelToChange, - waitForCellToHaveOutput + waitForCellHavingOutput } from './helper'; import { traceInfoIfCI } from '../../../client/common/logger'; @@ -103,7 +103,7 @@ suite('sys.path in Python Kernels', function () { await Promise.all([ runAllCellsInActiveNotebook(), waitForExecutionCompletedSuccessfully(cell), - waitForCellToHaveOutput(cell) + waitForCellHavingOutput(cell) ]); const output = Buffer.from(cell.outputs[0].items[0].data).toString().trim(); From e171c8772fe538b6cb6fc1b764282391178f1bd9 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 16 Mar 2022 06:20:27 +1100 Subject: [PATCH 08/10] Misc --- src/test/datascience/notebook/executionService.vscode.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/datascience/notebook/executionService.vscode.test.ts b/src/test/datascience/notebook/executionService.vscode.test.ts index 1aefcba7e95c..70da3244d45c 100644 --- a/src/test/datascience/notebook/executionService.vscode.test.ts +++ b/src/test/datascience/notebook/executionService.vscode.test.ts @@ -420,9 +420,9 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { await Promise.all([ runAllCellsInActiveNotebook(), waitForCellExecutionToComplete(cell1), - waitForCellHavingOutput(cell1), waitForCondition( async () => { + // Sometimes the cell can fail execution (IPython can sometimes throw an error). const output = getCellOutputs(cell1).trim(); if (output !== '' && output.length > 0) { return true; From 9ed14f5efd62834c3eda94d66517865ea9a1480c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 16 Mar 2022 06:21:06 +1100 Subject: [PATCH 09/10] Misc --- src/test/datascience/notebook/executionService.vscode.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/datascience/notebook/executionService.vscode.test.ts b/src/test/datascience/notebook/executionService.vscode.test.ts index 70da3244d45c..66b95746c4ac 100644 --- a/src/test/datascience/notebook/executionService.vscode.test.ts +++ b/src/test/datascience/notebook/executionService.vscode.test.ts @@ -440,7 +440,6 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { // On windows `!where python`, prints multiple items in the output (all executables found). const cell1Output = getCellOutputs(cell1); const shellExecutable = cell1Output - .trim() .split('\n') .filter((item) => item.length)[0] .trim(); From 48b79264e9c7b0dc0e0500e67b832daace67d643 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 16 Mar 2022 06:22:05 +1100 Subject: [PATCH 10/10] Misc --- src/test/datascience/notebook/executionService.vscode.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/datascience/notebook/executionService.vscode.test.ts b/src/test/datascience/notebook/executionService.vscode.test.ts index 66b95746c4ac..9638e7247ff3 100644 --- a/src/test/datascience/notebook/executionService.vscode.test.ts +++ b/src/test/datascience/notebook/executionService.vscode.test.ts @@ -463,11 +463,11 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () { // First path in PATH must be the directory where executable is located. assert.ok( areInterpreterPathsSame(shellExecutable.toLowerCase(), sysExecutable.toLowerCase()), - `Python paths do not match ${shellExecutable}, ${sysExecutable}. Output is (${ + `Python paths do not match ${shellExecutable}, ${sysExecutable}. Output is (${cell1Output}, ${ cell1.outputs.length }) (${cell1.outputs .map((item) => item.items.map((i) => i.mime).join(',')) - .join('#')})${cell1Output}, error is ${errorOutput}` + .join('#')}), error is ${errorOutput}` ); }); test('Testing streamed output', async () => {