Skip to content

Commit

Permalink
Re-enable and Fix interrupt tests (#5796)
Browse files Browse the repository at this point in the history
  • Loading branch information
DonJayamanne authored May 6, 2021
1 parent 8155577 commit aaa6fe3
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ export class NativeEditorCommandListener implements IDataScienceCommandListener
commandManager.registerCommand(Commands.NotebookEditorRemoveAllCells, () => this.removeAllCells())
);
this.disposableRegistry.push(
commandManager.registerCommand(Commands.NotebookEditorInterruptKernel, (document: Uri | undefined) =>
this.interruptKernel(document)
commandManager.registerCommand(Commands.NotebookEditorInterruptKernel, (notebookUri: Uri | undefined) =>
this.interruptKernel(notebookUri)
)
);
this.disposableRegistry.push(
Expand Down Expand Up @@ -99,19 +99,21 @@ export class NativeEditorCommandListener implements IDataScienceCommandListener
}
}

private interruptKernel(document: Uri | undefined) {
private interruptKernel(notebookUri: Uri | undefined) {
// `document` may be undefined if this command is invoked from the command palette.
if (document) {
traceInfo(`Interrupt requested for ${document.toString()} in nativeEditorCommandListener`);
if (notebookUri) {
traceInfo(`Interrupt requested for ${notebookUri.toString()} in nativeEditorCommandListener`);
traceInfo(`this.provider.activeEditor?.file.toString() = ${this.provider.activeEditor?.file.toString()}`);
traceInfo(`this.provider.editors = ${this.provider.editors.map((item) => item.file.toString())}`);
const target =
this.provider.activeEditor?.file.toString() === document.toString()
this.provider.activeEditor?.file.toString() === notebookUri.toString()
? this.provider.activeEditor
: this.provider.editors.find((editor) => editor.file.toString() === document.toString());
: this.provider.editors.find((editor) => editor.file.toString() === notebookUri.toString());
if (target) {
target.interruptKernel().ignoreErrors();
} else {
traceInfo(
`Interrupt requested for ${document.toString()} in nativeEditorCommandListener & editor not found`
`Interrupt requested for ${notebookUri.toString()} in nativeEditorCommandListener & editor not found`
);
}
} else {
Expand Down
8 changes: 2 additions & 6 deletions src/test/datascience/notebook/executionService.vscode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,6 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () {
assertHasTextOutputInVSCode(displayCell, 'foo', 0, true);
});
test('Clearing output while executing will ensure output is cleared', async function () {
// https://github.com/microsoft/vscode-jupyter/issues/5713
// The pending cells are always timing out on interrupt, and it might not be up to us
// We handle it by asking the user to restart the kernel instead
return this.skip();
// Assume you are executing a cell that prints numbers 1-100.
// When printing number 50, you click clear.
// Cell output should now start printing output from 51 onwards, & not 1.
Expand Down Expand Up @@ -290,10 +286,10 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () {
);

// Interrupt the kernel).
traceInfo(`Interrupt requested for ${vscodeNotebook.activeNotebookEditor?.document?.uri} in test`);
traceInfo(`Interrupt requested for ${vscodeNotebook.activeNotebookEditor?.document?.uri.toString()} in test`);
await commands.executeCommand(
'jupyter.notebookeditor.interruptkernel',
vscodeNotebook.activeNotebookEditor?.document
vscodeNotebook.activeNotebookEditor?.document.uri
);
await waitForExecutionCompletedWithErrors(cell);

Expand Down
4 changes: 0 additions & 4 deletions src/test/datascience/notebook/interruptRestart.vscode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,6 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors (slow)',
//await vscEditor.kernel!.interrupt!(vscEditor.document);
});
test('Interrupt and running cells again should only run the necessary cells', async function () {
// https://github.com/microsoft/vscode-jupyter/issues/5713
// The pending cells are always timing out on interrupt, and it might not be up to us
// We handle it by asking the user to restart the kernel instead
return this.skip();
// Interrupts on windows doesn't work well, not as well as on Unix.
// This is how Python works, hence this test is better tested on Unix OS.
// No need to test remote as this is a test of status (fewer slower tests is better).
Expand Down

0 comments on commit aaa6fe3

Please sign in to comment.