Skip to content

Commit 28e0350

Browse files
authored
Fail on cancellation signal when providing TensorBoard codelenses (microsoft#15332)
1 parent 5264b20 commit 28e0350

4 files changed

Lines changed: 48 additions & 18 deletions

File tree

src/client/tensorBoard/nbextensionCodeLensProvider.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import { inject, injectable } from 'inversify';
55
import { once } from 'lodash';
6-
import { CodeLens, Command, languages, Position, Range, TextDocument } from 'vscode';
6+
import { CancellationToken, CodeLens, Command, languages, Position, Range, TextDocument } from 'vscode';
77
import { IExtensionSingleActivationService } from '../activation/types';
88
import { Commands, NotebookCellScheme, PYTHON_LANGUAGE } from '../common/constants';
99
import { NativeTensorBoard } from '../common/experiments/groups';
@@ -46,7 +46,7 @@ export class TensorBoardNbextensionCodeLensProvider implements IExtensionSingleA
4646
}
4747
}
4848

49-
public provideCodeLenses(document: TextDocument): CodeLens[] {
49+
public provideCodeLenses(document: TextDocument, cancelToken: CancellationToken): CodeLens[] {
5050
const command: Command = {
5151
title: TensorBoard.launchNativeTensorBoardSessionCodeLens(),
5252
command: Commands.LaunchTensorBoard,
@@ -56,6 +56,9 @@ export class TensorBoardNbextensionCodeLensProvider implements IExtensionSingleA
5656
};
5757
const codelenses: CodeLens[] = [];
5858
for (let index = 0; index < document.lineCount; index += 1) {
59+
if (cancelToken.isCancellationRequested) {
60+
return codelenses;
61+
}
5962
const line = document.lineAt(index);
6063
if (containsNotebookExtension([line.text])) {
6164
const range = new Range(new Position(line.lineNumber, 0), new Position(line.lineNumber, 1));

src/client/tensorBoard/tensorBoardImportCodeLensProvider.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
import { inject, injectable } from 'inversify';
55
import { once } from 'lodash';
6-
import { CodeLens, Command, languages, Position, Range, TextDocument } from 'vscode';
6+
import { CancellationToken, CodeLens, Command, languages, Position, Range, TextDocument } from 'vscode';
77
import { IExtensionSingleActivationService } from '../activation/types';
88
import { Commands, PYTHON } from '../common/constants';
99
import { NativeTensorBoard } from '../common/experiments/groups';
@@ -33,7 +33,7 @@ export class TensorBoardImportCodeLensProvider implements IExtensionSingleActiva
3333
}
3434

3535
// eslint-disable-next-line class-methods-use-this
36-
public provideCodeLenses(document: TextDocument): CodeLens[] {
36+
public provideCodeLenses(document: TextDocument, cancelToken: CancellationToken): CodeLens[] {
3737
const command: Command = {
3838
title: TensorBoard.launchNativeTensorBoardSessionCodeLens(),
3939
command: Commands.LaunchTensorBoard,
@@ -43,6 +43,9 @@ export class TensorBoardImportCodeLensProvider implements IExtensionSingleActiva
4343
};
4444
const codelenses: CodeLens[] = [];
4545
for (let index = 0; index < document.lineCount; index += 1) {
46+
if (cancelToken.isCancellationRequested) {
47+
return codelenses;
48+
}
4649
const line = document.lineAt(index);
4750
if (containsTensorBoardImport([line.text])) {
4851
const range = new Range(new Position(line.lineNumber, 0), new Position(line.lineNumber, 1));

src/test/tensorBoard/nbextensionCodeLensProvider.unit.test.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import { assert } from 'chai';
55
import { mock } from 'ts-mockito';
6+
import { CancellationTokenSource } from 'vscode';
67
import { ExperimentService } from '../../client/common/experiments/service';
78
import { IExperimentService } from '../../client/common/types';
89
import { TensorBoardNbextensionCodeLensProvider } from '../../client/tensorBoard/nbextensionCodeLensProvider';
@@ -11,32 +12,43 @@ import { MockDocument } from '../startPage/mockDocument';
1112
suite('TensorBoard nbextension code lens provider', () => {
1213
let experimentService: IExperimentService;
1314
let codeLensProvider: TensorBoardNbextensionCodeLensProvider;
15+
let cancelTokenSource: CancellationTokenSource;
1416

1517
setup(() => {
1618
experimentService = mock(ExperimentService);
1719
codeLensProvider = new TensorBoardNbextensionCodeLensProvider(experimentService, []);
20+
cancelTokenSource = new CancellationTokenSource();
21+
});
22+
teardown(() => {
23+
cancelTokenSource.dispose();
1824
});
1925

2026
test('Provide code lens for Python notebook loading tensorboard nbextension', async () => {
2127
const document = new MockDocument('a=1\n%load_ext tensorboard', 'foo.ipynb', async () => true);
22-
const codeActions = codeLensProvider.provideCodeLenses(document);
23-
assert.ok(codeActions.length > 0, 'Failed to provide code lens for file loading tensorboard nbextension');
28+
const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token);
29+
assert.ok(codeLens.length > 0, 'Failed to provide code lens for file loading tensorboard nbextension');
2430
});
2531
test('Provide code lens for Python notebook launching tensorboard nbextension', async () => {
2632
const document = new MockDocument('a=1\n%tensorboard --logdir logs/fit', 'foo.ipynb', async () => true);
27-
const codeActions = codeLensProvider.provideCodeLenses(document);
28-
assert.ok(codeActions.length > 0, 'Failed to provide code lens for file loading tensorboard nbextension');
33+
const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token);
34+
assert.ok(codeLens.length > 0, 'Failed to provide code lens for file loading tensorboard nbextension');
35+
});
36+
test('Fails when cancellation is signaled', () => {
37+
const document = new MockDocument('a=1\n%tensorboard --logdir logs/fit', 'foo.ipynb', async () => true);
38+
cancelTokenSource.cancel();
39+
const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token);
40+
assert.ok(codeLens.length === 0, 'Provided codelens even after cancellation was requested');
2941
});
3042
// Can't verify these cases without running in vscode as we depend on vscode to not call us
3143
// based on the DocumentSelector we provided. See nbExtensionCodeLensProvider.test.ts for that.
3244
// test('Does not provide code lens for Python file loading tensorboard nbextension', async () => {
3345
// const document = new MockDocument('a=1\n%load_ext tensorboard', 'foo.py', async () => true);
34-
// const codeActions = codeLensProvider.provideCodeLenses(document);
35-
// assert.ok(codeActions.length === 0, 'Provided code lens for Python file loading tensorboard nbextension');
46+
// const codeLens = codeLensProvider.provideCodeLenses(document);
47+
// assert.ok(codeLens.length === 0, 'Provided code lens for Python file loading tensorboard nbextension');
3648
// });
3749
// test('Does not provide code lens for Python file launching tensorboard nbextension', async () => {
3850
// const document = new MockDocument('a=1\n%tensorboard --logdir logs/fit', 'foo.py', async () => true);
39-
// const codeActions = codeLensProvider.provideCodeLenses(document);
40-
// assert.ok(codeActions.length === 0, 'Provided code lens for Python file loading tensorboard nbextension');
51+
// const codeLens = codeLensProvider.provideCodeLenses(document);
52+
// assert.ok(codeLens.length === 0, 'Provided code lens for Python file loading tensorboard nbextension');
4153
// });
4254
});

src/test/tensorBoard/tensorBoardImportCodeLensProvider.unit.test.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import { assert } from 'chai';
55
import { mock } from 'ts-mockito';
6+
import { CancellationTokenSource } from 'vscode';
67
import { ExperimentService } from '../../client/common/experiments/service';
78
import { IExperimentService } from '../../client/common/types';
89
import { TensorBoardImportCodeLensProvider } from '../../client/tensorBoard/tensorBoardImportCodeLensProvider';
@@ -11,26 +12,37 @@ import { MockDocument } from '../startPage/mockDocument';
1112
suite('TensorBoard import code lens provider', () => {
1213
let experimentService: IExperimentService;
1314
let codeLensProvider: TensorBoardImportCodeLensProvider;
15+
let cancelTokenSource: CancellationTokenSource;
1416

1517
setup(() => {
1618
experimentService = mock(ExperimentService);
1719
codeLensProvider = new TensorBoardImportCodeLensProvider(experimentService, []);
20+
cancelTokenSource = new CancellationTokenSource();
21+
});
22+
teardown(() => {
23+
cancelTokenSource.dispose();
1824
});
1925
['tensorboard', 'tensorboardX'].forEach((name) => {
2026
test(`Provides code lens for Python files importing ${name}`, () => {
2127
const document = new MockDocument(`import ${name}`, 'foo.py', async () => true);
22-
const codeActions = codeLensProvider.provideCodeLenses(document);
23-
assert.ok(codeActions.length > 0, `Failed to provide code lens for file containing ${name} import`);
28+
const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token);
29+
assert.ok(codeLens.length > 0, `Failed to provide code lens for file containing ${name} import`);
2430
});
2531
test(`Provides code lens for Python ipynbs importing ${name}`, () => {
2632
const document = new MockDocument(`import ${name}`, 'foo.ipynb', async () => true);
27-
const codeActions = codeLensProvider.provideCodeLenses(document);
28-
assert.ok(codeActions.length > 0, `Failed to provide code lens for ipynb containing ${name} import`);
33+
const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token);
34+
assert.ok(codeLens.length > 0, `Failed to provide code lens for ipynb containing ${name} import`);
35+
});
36+
test('Fails when cancellation is signaled', () => {
37+
const document = new MockDocument(`import ${name}`, 'foo.py', async () => true);
38+
cancelTokenSource.cancel();
39+
const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token);
40+
assert.ok(codeLens.length === 0, 'Provided codelens even after cancellation was requested');
2941
});
3042
});
3143
test('Does not provide code lens if no matching import', () => {
3244
const document = new MockDocument('import foo', 'foo.ipynb', async () => true);
33-
const codeActions = codeLensProvider.provideCodeLenses(document);
34-
assert.ok(codeActions.length === 0, 'Provided code lens for file without tensorboard import');
45+
const codeLens = codeLensProvider.provideCodeLenses(document, cancelTokenSource.token);
46+
assert.ok(codeLens.length === 0, 'Provided code lens for file without tensorboard import');
3547
});
3648
});

0 commit comments

Comments
 (0)