Skip to content

Commit 45a456b

Browse files
DonJayamanneDonJayamanne
authored
fix #1041 when debugging a test do not cancel it when re-discovering tests (DonJayamanne#1354)
* fix 1041 when debugging a test do not cancel it when re-discovering tests * create enum for creation of cancellation token * dispose correct cancellationToken
1 parent 1e54bd9 commit 45a456b

5 files changed

Lines changed: 72 additions & 43 deletions

File tree

‎src/client/unittests/common/baseTestManager.ts‎

Lines changed: 55 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,25 @@ import { resetTestResults, displayTestErrorMessage, storeDiscoveredTests } from
55
import { Installer, Product } from '../../common/installer';
66
import { isNotInstalledError } from '../../common/helpers';
77

8+
enum CancellationTokenType {
9+
testDicovery,
10+
testRunner
11+
}
12+
813
export abstract class BaseTestManager {
914
private tests: Tests;
1015
private _status: TestStatus = TestStatus.Unknown;
11-
private cancellationTokenSource: vscode.CancellationTokenSource;
16+
private testDiscoveryCancellationTokenSource: vscode.CancellationTokenSource;
17+
private testRunnerCancellationTokenSource: vscode.CancellationTokenSource;
1218
private installer: Installer;
13-
protected get cancellationToken(): vscode.CancellationToken {
14-
if (this.cancellationTokenSource) {
15-
return this.cancellationTokenSource.token;
19+
protected get testDiscoveryCancellationToken(): vscode.CancellationToken {
20+
if (this.testDiscoveryCancellationTokenSource) {
21+
return this.testDiscoveryCancellationTokenSource.token;
22+
}
23+
}
24+
protected get testRunnerCancellationToken(): vscode.CancellationToken {
25+
if (this.testRunnerCancellationTokenSource) {
26+
return this.testRunnerCancellationTokenSource.token;
1627
}
1728
}
1829
public dispose() {
@@ -21,8 +32,11 @@ export abstract class BaseTestManager {
2132
return this._status;
2233
}
2334
public stop() {
24-
if (this.cancellationTokenSource) {
25-
this.cancellationTokenSource.cancel();
35+
if (this.testDiscoveryCancellationTokenSource) {
36+
this.testDiscoveryCancellationTokenSource.cancel();
37+
}
38+
if (this.testRunnerCancellationTokenSource) {
39+
this.testRunnerCancellationTokenSource.cancel();
2640
}
2741
}
2842
constructor(private testProvider: string, private product: Product, protected rootDirectory: string, protected outputChannel: vscode.OutputChannel) {
@@ -40,18 +54,29 @@ export abstract class BaseTestManager {
4054

4155
resetTestResults(this.tests);
4256
}
43-
private createCancellationToken() {
44-
this.disposeCancellationToken();
45-
this.cancellationTokenSource = new vscode.CancellationTokenSource();
57+
private createCancellationToken(tokenType: CancellationTokenType) {
58+
this.disposeCancellationToken(tokenType);
59+
if (tokenType === CancellationTokenType.testDicovery) {
60+
this.testDiscoveryCancellationTokenSource = new vscode.CancellationTokenSource();
61+
} else {
62+
this.testRunnerCancellationTokenSource = new vscode.CancellationTokenSource();
63+
}
4664
}
47-
private disposeCancellationToken() {
48-
if (this.cancellationTokenSource) {
49-
this.cancellationTokenSource.dispose();
65+
private disposeCancellationToken(tokenType: CancellationTokenType) {
66+
if (tokenType === CancellationTokenType.testDicovery) {
67+
if (this.testDiscoveryCancellationTokenSource) {
68+
this.testDiscoveryCancellationTokenSource.dispose();
69+
}
70+
this.testDiscoveryCancellationTokenSource = null;
71+
} else {
72+
if (this.testRunnerCancellationTokenSource) {
73+
this.testRunnerCancellationTokenSource.dispose();
74+
}
75+
this.testRunnerCancellationTokenSource = null;
5076
}
51-
this.cancellationTokenSource = null;
5277
}
5378
private discoverTestsPromise: Promise<Tests>;
54-
discoverTests(ignoreCache: boolean = false, quietMode: boolean = false): Promise<Tests> {
79+
discoverTests(ignoreCache: boolean = false, quietMode: boolean = false, isUserInitiated: boolean = true): Promise<Tests> {
5580
if (this.discoverTestsPromise) {
5681
return this.discoverTestsPromise;
5782
}
@@ -61,8 +86,10 @@ export abstract class BaseTestManager {
6186
return Promise.resolve(this.tests);
6287
}
6388
this._status = TestStatus.Discovering;
64-
65-
this.createCancellationToken();
89+
if (isUserInitiated) {
90+
this.stop();
91+
}
92+
this.createCancellationToken(CancellationTokenType.testDicovery);
6693
return this.discoverTestsPromise = this.discoverTestsImpl(ignoreCache)
6794
.then(tests => {
6895
this.tests = tests;
@@ -85,7 +112,7 @@ export abstract class BaseTestManager {
85112
displayTestErrorMessage('There were some errors in disovering unit tests');
86113
}
87114
storeDiscoveredTests(tests);
88-
this.disposeCancellationToken();
115+
this.disposeCancellationToken(CancellationTokenType.testDicovery);
89116

90117
return tests;
91118
}).catch(reason => {
@@ -95,7 +122,7 @@ export abstract class BaseTestManager {
95122

96123
this.tests = null;
97124
this.discoverTestsPromise = null;
98-
if (this.cancellationToken && this.cancellationToken.isCancellationRequested) {
125+
if (this.testDiscoveryCancellationToken && this.testDiscoveryCancellationToken.isCancellationRequested) {
99126
reason = CANCELLATION_REASON;
100127
this._status = TestStatus.Idle;
101128
}
@@ -105,7 +132,7 @@ export abstract class BaseTestManager {
105132
this.outputChannel.appendLine('' + reason);
106133
}
107134
storeDiscoveredTests(null);
108-
this.disposeCancellationToken();
135+
this.disposeCancellationToken(CancellationTokenType.testDicovery);
109136
return Promise.reject(reason);
110137
});
111138
}
@@ -144,14 +171,15 @@ export abstract class BaseTestManager {
144171
}
145172

146173
this._status = TestStatus.Running;
147-
this.createCancellationToken();
174+
this.stop();
175+
this.createCancellationToken(CancellationTokenType.testDicovery);
148176
// If running failed tests, then don't clear the previously build UnitTests
149177
// If we do so, then we end up re-discovering the unit tests and clearing previously cached list of failed tests
150178
// Similarly, if running a specific test or test file, don't clear the cache (possible tests have some state information retained)
151179
const clearDiscoveredTestCache = runFailedTests || moreInfo.Run_Specific_File || moreInfo.Run_Specific_Class || moreInfo.Run_Specific_Function ? false : true;
152-
return this.discoverTests(clearDiscoveredTestCache, true)
180+
return this.discoverTests(clearDiscoveredTestCache, true, true)
153181
.catch(reason => {
154-
if (this.cancellationToken && this.cancellationToken.isCancellationRequested) {
182+
if (this.testDiscoveryCancellationToken && this.testDiscoveryCancellationToken.isCancellationRequested) {
155183
return Promise.reject<Tests>(reason);
156184
}
157185
displayTestErrorMessage('Errors in discovering tests, continuing with tests');
@@ -161,22 +189,23 @@ export abstract class BaseTestManager {
161189
};
162190
})
163191
.then(tests => {
192+
this.createCancellationToken(CancellationTokenType.testRunner);
164193
return this.runTestImpl(tests, testsToRun, runFailedTests, debug);
165194
}).then(() => {
166195
this._status = TestStatus.Idle;
167-
this.disposeCancellationToken();
196+
this.disposeCancellationToken(CancellationTokenType.testRunner);
168197
return this.tests;
169198
}).catch(reason => {
170-
if (this.cancellationToken && this.cancellationToken.isCancellationRequested) {
199+
if (this.testRunnerCancellationToken && this.testRunnerCancellationToken.isCancellationRequested) {
171200
reason = CANCELLATION_REASON;
172201
this._status = TestStatus.Idle;
173202
}
174203
else {
175204
this._status = TestStatus.Error;
176205
}
177-
this.disposeCancellationToken();
206+
this.disposeCancellationToken(CancellationTokenType.testRunner);
178207
return Promise.reject<Tests>(reason);
179208
});
180209
}
181210
abstract runTestImpl(tests: Tests, testsToRun?: TestsToRun, runFailedTests?: boolean, debug?: boolean): Promise<any>;
182-
}
211+
}

‎src/client/unittests/main.ts‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export function activate(context: vscode.ExtensionContext, outputChannel: vscode
4040
if (settings.unitTest.nosetestsEnabled || settings.unitTest.pyTestEnabled || settings.unitTest.unittestEnabled) {
4141
// Ignore the exceptions returned
4242
// This function is invoked via a command which will be invoked else where in the extension
43-
discoverTests(true).catch(() => {
43+
discoverTests(true, false).catch(() => {
4444
// Ignore the errors
4545
});
4646
}
@@ -61,7 +61,7 @@ async function onDocumentSaved(doc: vscode.TextDocument): Promise<void> {
6161
return;
6262
}
6363

64-
let tests = await testManager.discoverTests(false, true);
64+
let tests = await testManager.discoverTests(false, true, false);
6565
if (!tests || !Array.isArray(tests.testFiles) || tests.testFiles.length === 0) {
6666
return;
6767
}
@@ -72,7 +72,7 @@ async function onDocumentSaved(doc: vscode.TextDocument): Promise<void> {
7272
if (timeoutId) {
7373
clearTimeout(timeoutId);
7474
}
75-
timeoutId = setTimeout(() => { discoverTests(true); }, 1000);
75+
timeoutId = setTimeout(() => { discoverTests(true, false); }, 1000);
7676
}
7777

7878
function dispose() {
@@ -91,7 +91,7 @@ function registerCommands(): vscode.Disposable[] {
9191
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Discover, () => {
9292
// Ignore the exceptions returned
9393
// This command will be invoked else where in the extension
94-
discoverTests(true).catch(() => { return null; });
94+
discoverTests(true, true).catch(() => { return null; });
9595
}));
9696
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Run_Failed, () => runTestsImpl(true)));
9797
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Run, (testId) => runTestsImpl(testId)));
@@ -134,7 +134,7 @@ function selectAndRunTestMethod(debug?: boolean) {
134134
if (!testManager) {
135135
return displayTestFrameworkError(outChannel);
136136
}
137-
testManager.discoverTests(true, true).then(() => {
137+
testManager.discoverTests(true, true, true).then(() => {
138138
const tests = getDiscoveredTests();
139139
testDisplay = testDisplay ? testDisplay : new TestDisplay();
140140
testDisplay.selectTestFunction(getTestWorkingDirectory(), tests).then(testFn => {
@@ -147,7 +147,7 @@ function selectAndRunTestFile() {
147147
if (!testManager) {
148148
return displayTestFrameworkError(outChannel);
149149
}
150-
testManager.discoverTests(true, true).then(() => {
150+
testManager.discoverTests(true, true, true).then(() => {
151151
const tests = getDiscoveredTests();
152152
testDisplay = testDisplay ? testDisplay : new TestDisplay();
153153
testDisplay.selectTestFile(getTestWorkingDirectory(), tests).then(testFile => {
@@ -164,7 +164,7 @@ function runCurrentTestFile() {
164164
if (!testManager) {
165165
return displayTestFrameworkError(outChannel);
166166
}
167-
testManager.discoverTests(true, true).then(() => {
167+
testManager.discoverTests(true, true, true).then(() => {
168168
const tests = getDiscoveredTests();
169169
const testFiles = tests.testFiles.filter(testFile => {
170170
return testFile.fullPath === currentFilePath;
@@ -225,7 +225,7 @@ function onConfigChanged() {
225225

226226
// No need to display errors
227227
if (settings.unitTest.nosetestsEnabled || settings.unitTest.pyTestEnabled || settings.unitTest.unittestEnabled) {
228-
discoverTests(true);
228+
discoverTests(true, false);
229229
}
230230
}
231231
function getTestRunner() {
@@ -248,7 +248,7 @@ function stopTests() {
248248
testManager.stop();
249249
}
250250
}
251-
function discoverTests(ignoreCache?: boolean) {
251+
function discoverTests(ignoreCache?: boolean, isUserInitiated?: boolean) {
252252
let testManager = getTestRunner();
253253
if (!testManager) {
254254
displayTestFrameworkError(outChannel);
@@ -257,7 +257,7 @@ function discoverTests(ignoreCache?: boolean) {
257257

258258
if (testManager && (testManager.status !== TestStatus.Discovering && testManager.status !== TestStatus.Running)) {
259259
testResultDisplay = testResultDisplay ? testResultDisplay : new TestResultDisplay(outChannel, onDidChange);
260-
return testResultDisplay.DisplayDiscoverStatus(testManager.discoverTests(ignoreCache));
260+
return testResultDisplay.DisplayDiscoverStatus(testManager.discoverTests(ignoreCache, false, isUserInitiated));
261261
}
262262
else {
263263
return Promise.resolve(null);
@@ -317,4 +317,4 @@ function runTestsImpl(arg?: vscode.Uri | TestsToRun | boolean | FlattenedTestFun
317317
});
318318

319319
testResultDisplay.DisplayProgressStatus(runPromise, debug);
320-
}
320+
}

‎src/client/unittests/nosetest/main.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export class TestManager extends BaseTestManager {
1616
}
1717
discoverTestsImpl(ignoreCache: boolean): Promise<Tests> {
1818
let args = settings.unitTest.nosetestArgs.slice(0);
19-
return discoverTests(this.rootDirectory, args, this.cancellationToken, ignoreCache, this.outputChannel);
19+
return discoverTests(this.rootDirectory, args, this.testDiscoveryCancellationToken, ignoreCache, this.outputChannel);
2020
}
2121
runTestImpl(tests: Tests, testsToRun?: TestsToRun, runFailedTests?: boolean, debug?: boolean): Promise<any> {
2222
let args = settings.unitTest.nosetestArgs.slice(0);
@@ -26,6 +26,6 @@ export class TestManager extends BaseTestManager {
2626
if (!runFailedTests && args.indexOf('--with-id') === -1) {
2727
args.push('--with-id');
2828
}
29-
return runTest(this.rootDirectory, tests, args, testsToRun, this.cancellationToken, this.outputChannel, debug);
29+
return runTest(this.rootDirectory, tests, args, testsToRun, this.testRunnerCancellationToken, this.outputChannel, debug);
3030
}
3131
}

‎src/client/unittests/pytest/main.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ export class TestManager extends BaseTestManager {
1414
}
1515
discoverTestsImpl(ignoreCache: boolean): Promise<Tests> {
1616
let args = settings.unitTest.pyTestArgs.slice(0);
17-
return discoverTests(this.rootDirectory, args, this.cancellationToken, ignoreCache, this.outputChannel);
17+
return discoverTests(this.rootDirectory, args, this.testDiscoveryCancellationToken, ignoreCache, this.outputChannel);
1818
}
1919
runTestImpl(tests: Tests, testsToRun?: TestsToRun, runFailedTests?: boolean, debug?: boolean): Promise<any> {
2020
let args = settings.unitTest.pyTestArgs.slice(0);
2121
if (runFailedTests === true && args.indexOf('--lf') === -1 && args.indexOf('--last-failed') === -1) {
2222
args.push('--last-failed');
2323
}
24-
return runTest(this.rootDirectory, tests, args, testsToRun, this.cancellationToken, this.outputChannel, debug);
24+
return runTest(this.rootDirectory, tests, args, testsToRun, this.testRunnerCancellationToken, this.outputChannel, debug);
2525
}
2626
}

‎src/client/unittests/unittest/main.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export class TestManager extends BaseTestManager {
1616
}
1717
discoverTestsImpl(ignoreCache: boolean): Promise<Tests> {
1818
let args = settings.unitTest.unittestArgs.slice(0);
19-
return discoverTests(this.rootDirectory, args, this.cancellationToken, ignoreCache, this.outputChannel);
19+
return discoverTests(this.rootDirectory, args, this.testDiscoveryCancellationToken, ignoreCache, this.outputChannel);
2020
}
2121
runTestImpl(tests: Tests, testsToRun?: TestsToRun, runFailedTests?: boolean, debug?: boolean): Promise<any> {
2222
let args = settings.unitTest.unittestArgs.slice(0);
@@ -26,6 +26,6 @@ export class TestManager extends BaseTestManager {
2626
return fn.testFunction.status === TestStatus.Error || fn.testFunction.status === TestStatus.Fail;
2727
}).map(fn => fn.testFunction);
2828
}
29-
return runTest(this, this.rootDirectory, tests, args, testsToRun, this.cancellationToken, this.outputChannel, debug);
29+
return runTest(this, this.rootDirectory, tests, args, testsToRun, this.testRunnerCancellationToken, this.outputChannel, debug);
3030
}
3131
}

0 commit comments

Comments
 (0)