Skip to content

Commit 896b588

Browse files
elliott-beachDonJayamanne
elliott-beach
authored and
DonJayamanne
committed
Fix unhandled promise rejections (#702)
* fix uncaught promise rejections * fix uncaught promise rejections due to forOf * fix possible uncaught promise from var extraction * cover cancelling test discovery
1 parent 68441c1 commit 896b588

4 files changed

Lines changed: 51 additions & 14 deletions

File tree

‎src/test/interpreters/condaEnvFileService.test.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,15 +104,20 @@ suite('Interpreters from Conda Environments Text File', () => {
104104
const interpreterPaths = [
105105
path.join(environmentsPath, 'conda', 'envs', 'numpy')
106106
];
107+
const pythonPath = path.join(interpreterPaths[0], 'pythonPath');
107108
condaService.setup(c => c.condaEnvironmentsFile).returns(() => environmentsFilePath);
109+
condaService.setup(c => c.getInterpreterPath(TypeMoq.It.isAny())).returns(() => pythonPath);
110+
fileSystem.setup(fs => fs.fileExistsAsync(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve(true));
108111
fileSystem.setup(fs => fs.fileExistsAsync(TypeMoq.It.isValue(environmentsFilePath))).returns(() => Promise.resolve(true));
109112
fileSystem.setup(fs => fs.readFile(TypeMoq.It.isValue(environmentsFilePath))).returns(() => Promise.resolve(interpreterPaths.join(EOL)));
110113

111-
AnacondaCompanyNames.forEach(async companyDisplayName => {
114+
for (const companyName of AnacondaCompanyNames) {
115+
const versionWithCompanyName = `Mock Version :: ${companyName}`;
116+
interpreterVersion.setup(c => c.getVersion(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(versionWithCompanyName));
112117
const interpreters = await condaFileProvider.getInterpreters();
113118

114119
assert.equal(interpreters.length, 1, 'Incorrect number of entries');
115-
assert.equal(interpreters[0].displayName, `${AnacondaDisplayName} Mock Version (numpy)`, 'Incorrect display name');
116-
});
120+
assert.equal(interpreters[0].displayName, `${AnacondaDisplayName} Mock Version`, 'Incorrect display name');
121+
}
117122
});
118123
});

‎src/test/refactor/extension.refactor.extract.var.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,13 @@ suite('Variable Extraction', () => {
101101
test('Extract Variable', async () => {
102102
const startPos = new vscode.Position(234, 29);
103103
const endPos = new vscode.Position(234, 38);
104-
testingVariableExtraction(false, startPos, endPos);
104+
await testingVariableExtraction(false, startPos, endPos);
105105
});
106106

107107
test('Extract Variable fails if whole string not selected', async () => {
108108
const startPos = new vscode.Position(234, 20);
109109
const endPos = new vscode.Position(234, 38);
110-
testingVariableExtraction(true, startPos, endPos);
110+
await testingVariableExtraction(true, startPos, endPos);
111111
});
112112

113113
function testingVariableExtractionEndToEnd(shouldError: boolean, startPos: Position, endPos: Position) {

‎src/test/unittests/debugger.test.ts‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,30 @@ suite('Unit Tests - debugging', () => {
7171
assert.equal(tests.testFunctions.length, 2, 'Incorrect number of test functions');
7272
assert.equal(tests.testSuites.length, 2, 'Incorrect number of test suites');
7373

74+
const deferred = createDeferred<string>();
7475
const testFunction = [tests.testFunctions[0].testFunction];
75-
testManager.runTest(CommandSource.commandPalette, { testFunction }, false, true);
76-
const launched = await mockDebugLauncher.launched;
77-
assert.isTrue(launched, 'Debugger not launched');
76+
const runningPromise = testManager.runTest(CommandSource.commandPalette, { testFunction }, false, true);
77+
78+
// This promise should never resolve nor reject.
79+
runningPromise
80+
.then(() => deferred.reject('Debugger stopped when it shouldn\'t have'))
81+
.catch(error => deferred.reject(error));
82+
83+
mockDebugLauncher.launched
84+
.then((launched) => {
85+
if (launched) {
86+
deferred.resolve('');
87+
} else {
88+
deferred.reject('Debugger not launched');
89+
}
90+
}) .catch(error => deferred.reject(error));
91+
92+
await deferred.promise;
7893
}
7994

8095
test('Debugger should start (unittest)', async () => {
8196
await updateSetting('unitTest.unittestArgs', ['-s=./tests', '-p=test_*.py'], rootWorkspaceUri, configTarget);
82-
await testStartingDebugger('unittest');
97+
await testStartingDebugger('unittest');
8398
});
8499

85100
test('Debugger should start (pytest)', async () => {
@@ -105,9 +120,10 @@ suite('Unit Tests - debugging', () => {
105120
const launched = await mockDebugLauncher.launched;
106121
assert.isTrue(launched, 'Debugger not launched');
107122

108-
testManager.discoverTests(CommandSource.commandPalette, true, true, true);
109-
123+
const discoveryPromise = testManager.discoverTests(CommandSource.commandPalette, true, true, true);
110124
await expect(runningPromise).to.be.rejectedWith(CANCELLATION_REASON, 'Incorrect reason for ending the debugger');
125+
ioc.dispose(); // will cancel test discovery
126+
await expect(discoveryPromise).to.be.rejectedWith(CANCELLATION_REASON, 'Incorrect reason for ending the debugger');
111127
}
112128

113129
test('Debugger should stop when user invokes a test discovery (unittest)', async () => {
@@ -151,6 +167,7 @@ suite('Unit Tests - debugging', () => {
151167
runningPromise
152168
.then(() => 'Debugger stopped when it shouldn\'t have')
153169
.catch(() => 'Debugger crashed when it shouldn\'t have')
170+
// tslint:disable-next-line: no-floating-promises
154171
.then(error => {
155172
deferred.reject(error);
156173
});

‎src/test/unittests/stoppingDiscoverAndTest.test.ts‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { expect, use } from 'chai';
55
import * as chaiAsPromised from 'chai-as-promised';
66
import * as path from 'path';
77
import { Uri } from 'vscode';
8+
import {createDeferred} from '../../client/common/helpers';
89
import { Product } from '../../client/common/types';
910
import { CANCELLATION_REASON, CommandSource, UNITTEST_PROVIDER } from '../../client/unittests/common/constants';
1011
import { ITestDiscoveryService } from '../../client/unittests/common/types';
@@ -60,9 +61,23 @@ suite('Unit Tests Stopping Discovery and Runner', () => {
6061

6162
const discoveryPromise = mockTestManager.discoverTests(CommandSource.auto);
6263
mockTestManager.discoveryDeferred.resolve(EmptyTests);
63-
mockTestManager.runTest(CommandSource.ui);
64+
const runningPromise = mockTestManager.runTest(CommandSource.ui);
65+
const deferred = createDeferred<string>();
6466

65-
await expect(discoveryPromise).to.eventually.equal(EmptyTests);
67+
// This promise should never resolve nor reject.
68+
runningPromise
69+
.then(() => Promise.reject('Debugger stopped when it shouldn\'t have'))
70+
.catch(error => deferred.reject(error));
71+
72+
discoveryPromise.then(result => {
73+
if (result === EmptyTests) {
74+
deferred.resolve('');
75+
} else {
76+
deferred.reject('tests not empty');
77+
}
78+
}).catch(error => deferred.reject(error));
79+
80+
await deferred.promise;
6681
});
6782

6883
test('Discovering tests should stop running tests', async () => {
@@ -75,7 +90,7 @@ suite('Unit Tests Stopping Discovery and Runner', () => {
7590
await new Promise(resolve => setTimeout(resolve, 1000));
7691

7792
// User manually discovering tests will kill the existing test runner.
78-
mockTestManager.discoverTests(CommandSource.ui, true, false, true);
93+
await mockTestManager.discoverTests(CommandSource.ui, true, false, true);
7994
await expect(runPromise).to.eventually.be.rejectedWith(CANCELLATION_REASON);
8095
});
8196
});

0 commit comments

Comments
 (0)