Skip to content

Commit e44e746

Browse files
DonJayamanneDonJayamanne
authored
Updates to cell status displayed in VS Code Notebooks (#12126)
For #10496 Display success when successfully executed, error when there's an error else idle.
1 parent 3df06fe commit e44e746

10 files changed

+524
-17
lines changed

‎src/client/datascience/interactive-ipynb/nativeEditorStorage.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,8 @@ export class NativeEditorNotebookModel implements INotebookModel {
331331
private updateCellExecutionCount(cellId: string, executionCount?: number) {
332332
const index = this.cells.findIndex((v) => v.id === cellId);
333333
if (index >= 0) {
334-
this._state.cells[index].data.execution_count = (executionCount || 0) > 0 ? executionCount : null;
334+
this._state.cells[index].data.execution_count =
335+
typeof executionCount === 'number' && executionCount > 0 ? executionCount : null;
335336
return true;
336337
}
337338
return false;

‎src/client/datascience/notebook/executionHelpers.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import type { nbformat } from '@jupyterlab/coreutils';
77
import type { KernelMessage } from '@jupyterlab/services';
88
import { NotebookCell, NotebookCellRunState, NotebookDocument } from 'vscode';
9+
import { IBaseCellVSCodeMetadata } from '../../../../types/@jupyterlab_coreutils_nbformat';
910
import { createErrorOutput } from '../../../datascience-ui/common/cellFactory';
1011
import {
1112
INotebookModelCellExecutionCountChange,
@@ -138,3 +139,34 @@ export function updateCellOutput(notebookCellModel: ICell, outputs: nbformat.IOu
138139
};
139140
model.update(updateCell);
140141
}
142+
143+
export function updateCellMetadata(
144+
notebookCellModel: ICell,
145+
metadata: Partial<IBaseCellVSCodeMetadata>,
146+
model: INotebookModel
147+
) {
148+
const originalVscodeMetadata: IBaseCellVSCodeMetadata = notebookCellModel.data.metadata.vscode || {};
149+
// Update our model with the new metadata stored in jupyter.
150+
const newCell: ICell = {
151+
...notebookCellModel,
152+
data: {
153+
...notebookCellModel.data,
154+
metadata: {
155+
...notebookCellModel.data.metadata,
156+
vscode: {
157+
...originalVscodeMetadata,
158+
...metadata
159+
}
160+
}
161+
}
162+
};
163+
const updateCell: INotebookModelModifyChange = {
164+
kind: 'modify',
165+
newCells: [newCell],
166+
oldCells: [notebookCellModel],
167+
newDirty: true,
168+
oldDirty: model.isDirty === true,
169+
source: 'user'
170+
};
171+
model.update(updateCell);
172+
}

‎src/client/datascience/notebook/executionService.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,6 @@ export class NotebookExecutionService implements INotebookExecutionService {
155155
const observable = nb.executeObservable(cell.source, document.fileName, 0, cell.uri.fsPath, false);
156156
subscription = observable?.subscribe(
157157
(cells) => {
158-
if (wrappedToken.isCancellationRequested) {
159-
return;
160-
}
161158
const rawCellOutput = cells
162159
.filter((item) => item.id === cell.uri.fsPath)
163160
.flatMap((item) => (item.data.outputs as unknown) as nbformat.IOutput[])
@@ -183,11 +180,15 @@ export class NotebookExecutionService implements INotebookExecutionService {
183180
this.errorHandler.handleError((error as unknown) as Error).ignoreErrors();
184181
},
185182
() => {
183+
cell.metadata.lastRunDuration = stopWatch.elapsedTime;
186184
cell.metadata.runState = wrappedToken.isCancellationRequested
187185
? vscodeNotebookEnums.NotebookCellRunState.Idle
188186
: vscodeNotebookEnums.NotebookCellRunState.Success;
189-
cell.metadata.lastRunDuration = stopWatch.elapsedTime;
190187
cell.metadata.statusMessage = '';
188+
// If there are any errors in the cell, then change status to error.
189+
if (cell.outputs.some((output) => output.outputKind === vscodeNotebookEnums.CellOutputKind.Error)) {
190+
cell.metadata.runState = vscodeNotebookEnums.NotebookCellRunState.Error;
191+
}
191192
deferred.resolve();
192193
}
193194
);

‎src/client/datascience/notebook/helpers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ export function notebookModelToVSCNotebookData(model: INotebookModel): NotebookD
3737

3838
return {
3939
cells,
40-
languages: [PYTHON_LANGUAGE, MARKDOWN_LANGUAGE],
40+
languages: [PYTHON_LANGUAGE],
4141
metadata: {
4242
cellEditable: true,
4343
cellRunnable: true,

‎src/test/datascience/notebook/contentProvider.unit.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ suite('Data Science - NativeNotebook ContentProvider', () => {
6161
const notebook = await contentProvider.openNotebook(fileUri);
6262

6363
assert.isOk(notebook);
64-
assert.deepEqual(notebook.languages, [PYTHON_LANGUAGE, MARKDOWN_LANGUAGE]);
64+
assert.deepEqual(notebook.languages, [PYTHON_LANGUAGE]);
6565
assert.deepEqual(notebook.cells, [
6666
{
6767
cellKind: (vscodeNotebookEnums as any).CellKind.Code,

‎src/test/datascience/notebook/executionWithJupyter.ds.test.ts renamed to ‎src/test/datascience/notebook/executionService.ds.test.ts

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,18 @@
44
'use strict';
55

66
// tslint:disable:no-require-imports no-var-requires
7-
import { assert } from 'chai';
7+
import { assert, expect } from 'chai';
88
import * as sinon from 'sinon';
99
import { commands } from 'vscode';
10+
import { CellErrorOutput } from '../../../../typings/vscode-proposed';
1011
import { IVSCodeNotebook } from '../../../client/common/application/types';
1112
import { IDisposable } from '../../../client/common/types';
1213
import { INotebookEditorProvider } from '../../../client/datascience/types';
1314
import { IExtensionTestApi, waitForCondition } from '../../common';
1415
import { initialize } from '../../initialize';
1516
import {
1617
assertHasExecutionCompletedSuccessfully,
18+
assertHasExecutionCompletedWithErrors,
1719
assertHasTextOutputInICell,
1820
assertHasTextOutputInVSCode,
1921
canRunTests,
@@ -23,6 +25,8 @@ import {
2325
startJupyter,
2426
swallowSavingOfNotebooks
2527
} from './helper';
28+
// tslint:disable-next-line: no-var-requires no-require-imports
29+
const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed');
2630

2731
// tslint:disable: no-any no-invalid-this
2832
suite('DataScience - VSCode Notebook - (Execution)', function () {
@@ -113,4 +117,48 @@ suite('DataScience - VSCode Notebook - (Execution)', function () {
113117
assert.ok(vscCell[0].metadata.executionOrder, 'Execution count should be > 0');
114118
assert.equal(vscCell[1].metadata.executionOrder! - 1, vscCell[0].metadata.executionOrder!);
115119
});
120+
test('Verify metadata for successfully executed cell', async () => {
121+
await insertPythonCellAndWait('print("Foo Bar")', 0);
122+
const cell = vscodeNotebook.activeNotebookEditor?.document.cells![0]!;
123+
124+
await commands.executeCommand('notebook.execute');
125+
126+
// Wait till execution count changes and status is success.
127+
await waitForCondition(
128+
async () => assertHasExecutionCompletedSuccessfully(cell),
129+
15_000,
130+
'Cell did not get executed'
131+
);
132+
133+
expect(cell.metadata.executionOrder).to.be.greaterThan(0, 'Execution count should be > 0');
134+
expect(cell.metadata.runStartTime).to.be.greaterThan(0, 'Start time should be > 0');
135+
expect(cell.metadata.lastRunDuration).to.be.greaterThan(0, 'Duration should be > 0');
136+
assert.equal(cell.metadata.runState, vscodeNotebookEnums.NotebookCellRunState.Success, 'Incorrect State');
137+
assert.equal(cell.metadata.statusMessage, '', 'Incorrect Status message');
138+
});
139+
test('Verify output & metadata for executed cell with errors', async () => {
140+
await insertPythonCellAndWait('print(abcd)', 0);
141+
const cell = vscodeNotebook.activeNotebookEditor?.document.cells![0]!;
142+
143+
await commands.executeCommand('notebook.execute');
144+
145+
// Wait till execution count changes and status is error.
146+
await waitForCondition(
147+
async () => assertHasExecutionCompletedWithErrors(cell),
148+
15_000,
149+
'Cell did not get executed'
150+
);
151+
152+
assert.lengthOf(cell.outputs, 1, 'Incorrect output');
153+
const errorOutput = cell.outputs[0] as CellErrorOutput;
154+
assert.equal(errorOutput.outputKind, vscodeNotebookEnums.CellOutputKind.Error, 'Incorrect output');
155+
assert.isNotEmpty(errorOutput.ename, 'Incorrect ename');
156+
assert.isNotEmpty(errorOutput.evalue, 'Incorrect evalue');
157+
assert.isNotEmpty(errorOutput.traceback, 'Incorrect traceback');
158+
expect(cell.metadata.executionOrder).to.be.greaterThan(0, 'Execution count should be > 0');
159+
expect(cell.metadata.runStartTime).to.be.greaterThan(0, 'Start time should be > 0');
160+
expect(cell.metadata.lastRunDuration).to.be.greaterThan(0, 'Duration should be > 0');
161+
assert.equal(cell.metadata.runState, vscodeNotebookEnums.NotebookCellRunState.Error, 'Incorrect State');
162+
assert.equal(cell.metadata.statusMessage, '', 'Incorrect Status message');
163+
});
116164
});

‎src/test/datascience/notebook/helper.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import * as sinon from 'sinon';
1111
import * as tmp from 'tmp';
1212
import { commands } from 'vscode';
1313
import { NotebookCell } from '../../../../types/vscode-proposed';
14+
import { CellDisplayOutput } from '../../../../typings/vscode-proposed';
1415
import { IApplicationEnvironment, IVSCodeNotebook } from '../../../client/common/application/types';
1516
import { MARKDOWN_LANGUAGE, PYTHON_LANGUAGE } from '../../../client/common/constants';
1617
import { IDisposable } from '../../../client/common/types';
@@ -211,6 +212,12 @@ export function assertHasExecutionCompletedSuccessfully(cell: NotebookCell) {
211212
cell.metadata.runState === vscodeNotebookEnums.NotebookCellRunState.Success
212213
);
213214
}
215+
export function assertHasExecutionCompletedWithErrors(cell: NotebookCell) {
216+
return (
217+
(cell.metadata.executionOrder ?? 0) > 0 &&
218+
cell.metadata.runState === vscodeNotebookEnums.NotebookCellRunState.Error
219+
);
220+
}
214221
export function hasOutputInVSCode(cell: NotebookCell) {
215222
assert.ok(cell.outputs.length, 'No output');
216223
}
@@ -220,8 +227,8 @@ export function hasOutputInICell(cell: ICell) {
220227
export function assertHasTextOutputInVSCode(cell: NotebookCell, text: string, index: number, isExactMatch = true) {
221228
const cellOutputs = cell.outputs;
222229
assert.ok(cellOutputs, 'No output');
223-
assert.equal(cellOutputs[index].outputKind, vscodeNotebookEnums.CellOutputKind.Text, 'Incorrect output kind');
224-
const outputText = (cellOutputs[index] as any).text.trim();
230+
assert.equal(cellOutputs[index].outputKind, vscodeNotebookEnums.CellOutputKind.Rich, 'Incorrect output kind');
231+
const outputText = (cellOutputs[index] as CellDisplayOutput).data['text/plain'].trim();
225232
if (isExactMatch) {
226233
assert.equal(outputText, text, 'Incorrect output');
227234
} else {
@@ -241,3 +248,7 @@ export function assertVSCCellIsIdle(cell: NotebookCell) {
241248
assert.equal(cell.metadata.runState, vscodeNotebookEnums.NotebookCellRunState.Idle);
242249
return true;
243250
}
251+
export function assertVSCCellHasErrors(cell: NotebookCell) {
252+
assert.equal(cell.metadata.runState, vscodeNotebookEnums.NotebookCellRunState.Error);
253+
return true;
254+
}

‎src/test/datascience/notebook/helpers.unit.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ suite('Data Science - NativeNotebook helpers', () => {
4646
const notebook = notebookModelToVSCNotebookData((model as unknown) as INotebookModel);
4747

4848
assert.isOk(notebook);
49-
assert.deepEqual(notebook.languages, [PYTHON_LANGUAGE, MARKDOWN_LANGUAGE]);
49+
assert.deepEqual(notebook.languages, [PYTHON_LANGUAGE]);
5050
assert.deepEqual(notebook.cells, [
5151
{
5252
cellKind: vscodeNotebookEnums.CellKind.Code,

‎src/test/datascience/notebook/interrupRestart.ds.test.ts

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import { assert } from 'chai';
77
import * as sinon from 'sinon';
88
import { CancellationTokenSource, commands, NotebookEditor as VSCNotebookEditor } from 'vscode';
9+
import { CellErrorOutput, CellStreamOutput } from '../../../../typings/vscode-proposed';
910
import { IVSCodeNotebook } from '../../../client/common/application/types';
1011
import { IDisposable } from '../../../client/common/types';
1112
import { createDeferredFromPromise, sleep } from '../../../client/common/utils/async';
@@ -14,6 +15,7 @@ import { INotebookEditorProvider } from '../../../client/datascience/types';
1415
import { IExtensionTestApi, waitForCondition } from '../../common';
1516
import { initialize } from '../../initialize';
1617
import {
18+
assertVSCCellHasErrors,
1719
assertVSCCellIsIdle,
1820
assertVSCCellIsRunning,
1921
canRunTests,
@@ -24,6 +26,8 @@ import {
2426
startJupyter,
2527
swallowSavingOfNotebooks
2628
} from './helper';
29+
// tslint:disable-next-line: no-var-requires no-require-imports
30+
const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed');
2731

2832
// tslint:disable: no-any no-invalid-this
2933
/*
@@ -47,6 +51,7 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors', functio
4751
if (!(await canRunTests())) {
4852
return this.skip();
4953
}
54+
await closeNotebooksAndCleanUpAfterTests();
5055
await startJupyter();
5156
vscodeNotebook = api.serviceContainer.get<IVSCodeNotebook>(IVSCodeNotebook);
5257
editorProvider = api.serviceContainer.get<INotebookEditorProvider>(INotebookEditorProvider);
@@ -85,7 +90,7 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors', functio
8590
cancellation.cancel();
8691

8792
await waitForCondition(async () => deferred.completed, 5_000, 'Execution not cancelled');
88-
assertVSCCellIsIdle(cell);
93+
assertVSCCellHasErrors(cell);
8994
});
9095
test('Cancelling using VSC Command for cell (slow)', async function () {
9196
// Fails due to VSC bugs.
@@ -105,7 +110,7 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors', functio
105110
// Interrupt the kernel.
106111
await commands.executeCommand('notebook.cell.cancelExecution', cell);
107112

108-
await waitForCondition(async () => assertVSCCellIsIdle(cell), 1_000, 'Execution not cancelled');
113+
await waitForCondition(async () => assertVSCCellHasErrors(cell), 1_000, 'Execution not cancelled');
109114
});
110115
test('Cancelling using VSC Command in toolbar (slow)', async () => {
111116
await insertPythonCellAndWait('import time\nfor i in range(10000):\n print(i)\n time.sleep(0.1)', 0);
@@ -123,7 +128,7 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors', functio
123128
// Interrupt the kernel.
124129
await commands.executeCommand('notebook.cancelExecution');
125130

126-
await waitForCondition(async () => assertVSCCellIsIdle(cell), 1_000, 'Execution not cancelled');
131+
await waitForCondition(async () => assertVSCCellHasErrors(cell), 1_000, 'Execution not cancelled');
127132
});
128133
test('When running entire notebook, clicking VSCode Stop button should trigger a single interrupt, not one per cell', async () => {
129134
await insertPythonCellAndWait('import time\nfor i in range(10000):\n print(i)\n time.sleep(0.1)', 0);
@@ -151,11 +156,20 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors', functio
151156

152157
// Wait for ?s, and verify cells are not running.
153158
await waitForCondition(
154-
async () => assertVSCCellIsIdle(cell1) && assertVSCCellIsIdle(cell2) && assertVSCCellIsIdle(cell3),
159+
// First cell will have interrupt error and others will have just stopped even before they start.
160+
async () => assertVSCCellHasErrors(cell1) && assertVSCCellIsIdle(cell2) && assertVSCCellIsIdle(cell3),
155161
15_000,
156162
'Cells are still running'
157163
);
158164

165+
assert.lengthOf(cell1.outputs, 2, 'First cell should have two outputs (stream and error)');
166+
assert.lengthOf(cell2.outputs, 0, 'Should not have any output');
167+
assert.lengthOf(cell3.outputs, 0, 'Should not have any output');
168+
169+
const cell1Output1 = cell1.outputs[0] as CellStreamOutput;
170+
const cell1Output2 = cell1.outputs[1] as CellErrorOutput;
171+
assert.equal(cell1Output1.outputKind, vscodeNotebookEnums.CellOutputKind.Rich, 'Should be text output');
172+
assert.equal(cell1Output2.outputKind, vscodeNotebookEnums.CellOutputKind.Error, 'Should be error output');
159173
assert.equal(interrupt.callCount, 1, 'Interrupt should have been invoked only once');
160174
});
161175
test('Restarting kernel will cancel cell execution (slow)', async () => {
@@ -174,7 +188,7 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors', functio
174188
// Restart the kernel.
175189
const restartPromise = commands.executeCommand('python.datascience.notebookeditor.restartkernel');
176190

177-
await waitForCondition(async () => assertVSCCellIsIdle(cell), 1_000, 'Execution not cancelled');
191+
await waitForCondition(async () => assertVSCCellIsIdle(cell), 15_000, 'Execution not cancelled');
178192

179193
// Wait before we execute cells again.
180194
await restartPromise;
@@ -187,6 +201,6 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors', functio
187201
await commands.executeCommand('notebook.cancelExecution');
188202

189203
// Wait for ?s, and verify cells are not running.
190-
await waitForCondition(async () => assertVSCCellIsIdle(cell), 15_000, 'Cell is still running');
204+
await waitForCondition(async () => assertVSCCellHasErrors(cell), 15_000, 'Cell is still running');
191205
});
192206
});

0 commit comments

Comments
 (0)