Skip to content

Commit

Permalink
replicate solution for #1041 and #1354
Browse files Browse the repository at this point in the history
  • Loading branch information
@DonJayamanne
DonJayamanne committed Oct 29, 2017
1 parent c753dbe commit 57821f9
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 37 deletions.
4 changes: 2 additions & 2 deletions src/client/common/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ function handleResponse(file: string, includeErrorAsResponse: boolean, error: Er

// pylint:
// In the case of pylint we have some messages (such as config file not found and using default etc...) being returned in stderr
// These error messages are useless when using pylint
// These error messages are useless when using pylint
if (includeErrorAsResponse && (stdout.length > 0 || stderr.length > 0)) {
return Promise.resolve(stdout + '\n' + stderr);
}
Expand All @@ -189,7 +189,7 @@ function handlePythonModuleResponse(includeErrorAsResponse: boolean, error: Erro

// pylint:
// In the case of pylint we have some messages (such as config file not found and using default etc...) being returned in stderr
// These error messages are useless when using pylint
// These error messages are useless when using pylint
if (includeErrorAsResponse && (stdout.length > 0 || stderr.length > 0)) {
return Promise.resolve(stdout + '\n' + stderr);
}
Expand Down
70 changes: 47 additions & 23 deletions src/client/unittests/common/baseTestManager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ export abstract class BaseTestManager {
private tests: Tests;
// tslint:disable-next-line:variable-name
private _status: TestStatus = TestStatus.Unknown;
private cancellationTokenSource: vscode.CancellationTokenSource;
private discoveryCancellationTokenSource: vscode.CancellationTokenSource;
private testRunnerCancellationTokenSource: vscode.CancellationTokenSource;
private installer: Installer;
private discoverTestsPromise: Promise<Tests>;
constructor(private testProvider: string, private product: Product, protected rootDirectory: string,
Expand All @@ -25,8 +26,11 @@ export abstract class BaseTestManager {
this.settings = PythonSettings.getInstance(this.rootDirectory ? Uri.file(this.rootDirectory) : undefined);
this.workspace = workspace.getWorkspaceFolder(Uri.file(this.rootDirectory)).uri;
}
protected get cancellationToken(): vscode.CancellationToken | undefined {
return this.cancellationTokenSource ? this.cancellationTokenSource.token : undefined;
protected get testDiscoveryCancellationToken(): vscode.CancellationToken | undefined {
return this.discoveryCancellationTokenSource ? this.discoveryCancellationTokenSource.token : undefined;
}
protected get testRunnerCancellationToken(): vscode.CancellationToken | undefined {
return this.testRunnerCancellationTokenSource ? this.testRunnerCancellationTokenSource.token : undefined;
}
public dispose() {
this.stop();
Expand All @@ -39,8 +43,11 @@ export abstract class BaseTestManager {
return settings.unitTest.cwd && settings.unitTest.cwd.length > 0 ? settings.unitTest.cwd : this.rootDirectory;
}
public stop() {
if (this.cancellationTokenSource) {
this.cancellationTokenSource.cancel();
if (this.discoveryCancellationTokenSource) {
this.discoveryCancellationTokenSource.cancel();
}
if (this.testRunnerCancellationTokenSource) {
this.testRunnerCancellationTokenSource.cancel();
}
}
public reset() {
Expand All @@ -54,7 +61,7 @@ export abstract class BaseTestManager {

this.testResultsService.resetResults(this.tests);
}
public async discoverTests(ignoreCache: boolean = false, quietMode: boolean = false): Promise<Tests> {
public async discoverTests(ignoreCache: boolean = false, quietMode: boolean = false, userInitiated: boolean = false): Promise<Tests> {
if (this.discoverTestsPromise) {
return this.discoverTestsPromise;
}
Expand All @@ -65,7 +72,13 @@ export abstract class BaseTestManager {
}
this._status = TestStatus.Discovering;

this.createCancellationToken();
// If ignoreCache is true, its an indication of the fact that its a user invoked operation.
// Hence we can stop the debugger.
if (userInitiated) {
this.stop();
}

this.createCancellationTokens();
return this.discoverTestsPromise = this.discoverTestsImpl(ignoreCache)
.then(tests => {
this.tests = tests;
Expand All @@ -89,7 +102,7 @@ export abstract class BaseTestManager {
}
const wkspace = vscode.workspace.getWorkspaceFolder(vscode.Uri.file(this.rootDirectory)).uri;
this.testCollectionStorage.storeTests(wkspace, tests);
this.disposeCancellationToken();
this.disposeCancellationTokens();

return tests;
}).catch(reason => {
Expand All @@ -100,7 +113,7 @@ export abstract class BaseTestManager {

this.tests = null;
this.discoverTestsPromise = null;
if (this.cancellationToken && this.cancellationToken.isCancellationRequested) {
if (this.testDiscoveryCancellationToken && this.testDiscoveryCancellationToken.isCancellationRequested) {
reason = CANCELLATION_REASON;
this._status = TestStatus.Idle;
} else {
Expand All @@ -111,7 +124,7 @@ export abstract class BaseTestManager {
}
const wkspace = vscode.workspace.getWorkspaceFolder(vscode.Uri.file(this.rootDirectory)).uri;
this.testCollectionStorage.storeTests(wkspace, null);
this.disposeCancellationToken();
this.disposeCancellationTokens();
return Promise.reject(reason);
});
}
Expand Down Expand Up @@ -144,14 +157,16 @@ export abstract class BaseTestManager {
}

this._status = TestStatus.Running;
this.createCancellationToken();
this.stop();
this.createCancellationTokens(true);

// If running failed tests, then don't clear the previously build UnitTests
// If we do so, then we end up re-discovering the unit tests and clearing previously cached list of failed tests
// Similarly, if running a specific test or test file, don't clear the cache (possible tests have some state information retained)
const clearDiscoveredTestCache = runFailedTests || moreInfo.Run_Specific_File || moreInfo.Run_Specific_Class || moreInfo.Run_Specific_Function ? false : true;
return this.discoverTests(clearDiscoveredTestCache, true)
return this.discoverTests(clearDiscoveredTestCache, true, true)
.catch(reason => {
if (this.cancellationToken && this.cancellationToken.isCancellationRequested) {
if (this.testRunnerCancellationToken && this.testRunnerCancellationToken.isCancellationRequested) {
return Promise.reject<Tests>(reason);
}
displayTestErrorMessage('Errors in discovering tests, continuing with tests');
Expand All @@ -164,30 +179,39 @@ export abstract class BaseTestManager {
return this.runTestImpl(tests, testsToRun, runFailedTests, debug);
}).then(() => {
this._status = TestStatus.Idle;
this.disposeCancellationToken();
this.disposeCancellationTokens(true);
return this.tests;
}).catch(reason => {
if (this.cancellationToken && this.cancellationToken.isCancellationRequested) {
if (this.testRunnerCancellationToken && this.testRunnerCancellationToken.isCancellationRequested) {
reason = CANCELLATION_REASON;
this._status = TestStatus.Idle;
} else {
this._status = TestStatus.Error;
}
this.disposeCancellationToken();
this.disposeCancellationTokens(true);
return Promise.reject<Tests>(reason);
});
}
// tslint:disable-next-line:no-any
protected abstract runTestImpl(tests: Tests, testsToRun?: TestsToRun, runFailedTests?: boolean, debug?: boolean): Promise<any>;
protected abstract discoverTestsImpl(ignoreCache: boolean, debug?: boolean): Promise<Tests>;
private createCancellationToken() {
this.disposeCancellationToken();
this.cancellationTokenSource = new vscode.CancellationTokenSource();
private createCancellationTokens(createTestRunnerToken: boolean = false) {
this.disposeCancellationTokens(createTestRunnerToken);
this.discoveryCancellationTokenSource = new vscode.CancellationTokenSource();
if (createTestRunnerToken) {
this.testRunnerCancellationTokenSource = new vscode.CancellationTokenSource();
}
}
private disposeCancellationToken() {
if (this.cancellationTokenSource) {
this.cancellationTokenSource.dispose();
private disposeCancellationTokens(disposeTestRunnerToken: boolean = false) {
if (this.discoveryCancellationTokenSource) {
this.discoveryCancellationTokenSource.dispose();
}
this.discoveryCancellationTokenSource = null;
if (disposeTestRunnerToken) {
if (this.testRunnerCancellationTokenSource) {
this.testRunnerCancellationTokenSource.dispose();
}
this.testRunnerCancellationTokenSource = null;
}
this.cancellationTokenSource = null;
}
}
12 changes: 6 additions & 6 deletions src/client/unittests/main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ function registerCommands(): vscode.Disposable[] {
// Ignore the exceptions returned.
// This command will be invoked else where in the extension.
// tslint:disable-next-line:no-empty
discoverTests(resource, true).catch(() => { });
discoverTests(resource, true, true).catch(() => { });
}));
disposables.push(vscode.commands.registerCommand(constants.Commands.Tests_Run_Failed, () => runTestsImpl(undefined, undefined, true)));
// tslint:disable-next-line:no-unnecessary-callback-wrapper
Expand Down Expand Up @@ -146,7 +146,7 @@ async function selectAndRunTestMethod(debug?: boolean) {
return;
}
try {
await testManager.discoverTests(true, true);
await testManager.discoverTests(true, true, true);
} catch (ex) {
return;
}
Expand All @@ -166,7 +166,7 @@ async function selectAndRunTestFile() {
return;
}
try {
await testManager.discoverTests(true, true);
await testManager.discoverTests(true, true, true);
} catch (ex) {
return;
}
Expand All @@ -189,7 +189,7 @@ async function runCurrentTestFile() {
return;
}
try {
await testManager.discoverTests(true, true);
await testManager.discoverTests(true, true, true);
} catch (ex) {
return;
}
Expand Down Expand Up @@ -271,15 +271,15 @@ async function stopTests(resource: Uri) {
testManager.stop();
}
}
async function discoverTests(resource?: Uri, ignoreCache?: boolean) {
async function discoverTests(resource?: Uri, ignoreCache?: boolean, userInitiated?: boolean) {
const testManager = await getTestManager(true, resource);
if (!testManager) {
return;
}

if (testManager && (testManager.status !== TestStatus.Discovering && testManager.status !== TestStatus.Running)) {
testResultDisplay = testResultDisplay ? testResultDisplay : new TestResultDisplay(outChannel, onDidChange);
const discoveryPromise = testManager.discoverTests(ignoreCache);
const discoveryPromise = testManager.discoverTests(ignoreCache, false, userInitiated);
testResultDisplay.displayDiscoverStatus(discoveryPromise);
await discoveryPromise;
}
Expand Down
4 changes: 2 additions & 2 deletions src/client/unittests/nosetest/main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export class TestManager extends BaseTestManager {
}
public discoverTestsImpl(ignoreCache: boolean): Promise<Tests> {
const args = this.settings.unitTest.nosetestArgs.slice(0);
return discoverTests(this.rootDirectory, args, this.cancellationToken, ignoreCache, this.outputChannel, this.testsHelper);
return discoverTests(this.rootDirectory, args, this.testDiscoveryCancellationToken, ignoreCache, this.outputChannel, this.testsHelper);
}
// tslint:disable-next-line:no-any
public runTestImpl(tests: Tests, testsToRun?: TestsToRun, runFailedTests?: boolean, debug?: boolean): Promise<any> {
Expand All @@ -27,6 +27,6 @@ export class TestManager extends BaseTestManager {
if (!runFailedTests && args.indexOf('--with-id') === -1) {
args.push('--with-id');
}
return runTest(this.testResultsService, this.rootDirectory, tests, args, testsToRun, this.cancellationToken, this.outputChannel, debug);
return runTest(this.testResultsService, this.rootDirectory, tests, args, testsToRun, this.testRunnerCancellationToken, this.outputChannel, debug);
}
}
4 changes: 2 additions & 2 deletions src/client/unittests/pytest/main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ export class TestManager extends BaseTestManager {
}
public discoverTestsImpl(ignoreCache: boolean): Promise<Tests> {
const args = this.settings.unitTest.pyTestArgs.slice(0);
return discoverTests(this.rootDirectory, args, this.cancellationToken, ignoreCache, this.outputChannel, this.testsHelper);
return discoverTests(this.rootDirectory, args, this.testDiscoveryCancellationToken, ignoreCache, this.outputChannel, this.testsHelper);
}
public runTestImpl(tests: Tests, testsToRun?: TestsToRun, runFailedTests?: boolean, debug?: boolean): Promise<{}> {
const args = this.settings.unitTest.pyTestArgs.slice(0);
if (runFailedTests === true && args.indexOf('--lf') === -1 && args.indexOf('--last-failed') === -1) {
args.push('--last-failed');
}
return runTest(this.testResultsService, this.rootDirectory, tests, args, testsToRun, this.cancellationToken, this.outputChannel, debug);
return runTest(this.testResultsService, this.rootDirectory, tests, args, testsToRun, this.testRunnerCancellationToken, this.outputChannel, debug);
}
}
4 changes: 2 additions & 2 deletions src/client/unittests/unittest/main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export class TestManager extends BaseTestManager {
}
public discoverTestsImpl(ignoreCache: boolean): Promise<Tests> {
const args = this.settings.unitTest.unittestArgs.slice(0);
return discoverTests(this.rootDirectory, args, this.cancellationToken, ignoreCache, this.outputChannel, this.testsHelper);
return discoverTests(this.rootDirectory, args, this.testDiscoveryCancellationToken, ignoreCache, this.outputChannel, this.testsHelper);
}
public runTestImpl(tests: Tests, testsToRun?: TestsToRun, runFailedTests?: boolean, debug?: boolean): Promise<{}> {
const args = this.settings.unitTest.unittestArgs.slice(0);
Expand All @@ -27,6 +27,6 @@ export class TestManager extends BaseTestManager {
return fn.testFunction.status === TestStatus.Error || fn.testFunction.status === TestStatus.Fail;
}).map(fn => fn.testFunction);
}
return runTest(this, this.testResultsService, this.rootDirectory, tests, args, testsToRun, this.cancellationToken, this.outputChannel, debug);
return runTest(this, this.testResultsService, this.rootDirectory, tests, args, testsToRun, this.testRunnerCancellationToken, this.outputChannel, debug);
}
}

0 comments on commit 57821f9

Please sign in to comment.