Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unhandled promise rejections #702

Merged
merged 6 commits into from
Feb 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/test/interpreters/condaEnvFileService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,20 @@ suite('Interpreters from Conda Environments Text File', () => {
const interpreterPaths = [
path.join(environmentsPath, 'conda', 'envs', 'numpy')
];
const pythonPath = path.join(interpreterPaths[0], 'pythonPath');
condaService.setup(c => c.condaEnvironmentsFile).returns(() => environmentsFilePath);
condaService.setup(c => c.getInterpreterPath(TypeMoq.It.isAny())).returns(() => pythonPath);
fileSystem.setup(fs => fs.fileExistsAsync(TypeMoq.It.isValue(pythonPath))).returns(() => Promise.resolve(true));
fileSystem.setup(fs => fs.fileExistsAsync(TypeMoq.It.isValue(environmentsFilePath))).returns(() => Promise.resolve(true));
fileSystem.setup(fs => fs.readFile(TypeMoq.It.isValue(environmentsFilePath))).returns(() => Promise.resolve(interpreterPaths.join(EOL)));

AnacondaCompanyNames.forEach(async companyDisplayName => {
Copy link
Author

@elliott-beach elliott-beach Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forEach on an async function doesn't do anything.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see below for the fix to this entire test

for (const companyName of AnacondaCompanyNames) {
const versionWithCompanyName = `Mock Version :: ${companyName}`;
interpreterVersion.setup(c => c.getVersion(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(versionWithCompanyName));
const interpreters = await condaFileProvider.getInterpreters();

assert.equal(interpreters.length, 1, 'Incorrect number of entries');
assert.equal(interpreters[0].displayName, `${AnacondaDisplayName} Mock Version (numpy)`, 'Incorrect display name');
});
assert.equal(interpreters[0].displayName, `${AnacondaDisplayName} Mock Version`, 'Incorrect display name');
}
});
});
4 changes: 2 additions & 2 deletions src/test/refactor/extension.refactor.extract.var.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,13 @@ suite('Variable Extraction', () => {
test('Extract Variable', async () => {
const startPos = new vscode.Position(234, 29);
const endPos = new vscode.Position(234, 38);
testingVariableExtraction(false, startPos, endPos);
await testingVariableExtraction(false, startPos, endPos);
});

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

function testingVariableExtractionEndToEnd(shouldError: boolean, startPos: Position, endPos: Position) {
Expand Down
29 changes: 23 additions & 6 deletions src/test/unittests/debugger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,30 @@ suite('Unit Tests - debugging', () => {
assert.equal(tests.testFunctions.length, 2, 'Incorrect number of test functions');
assert.equal(tests.testSuites.length, 2, 'Incorrect number of test suites');

const deferred = createDeferred<string>();
Copy link
Author

@elliott-beach elliott-beach Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const testFunction = [tests.testFunctions[0].testFunction];
testManager.runTest(CommandSource.commandPalette, { testFunction }, false, true);
const launched = await mockDebugLauncher.launched;
assert.isTrue(launched, 'Debugger not launched');
const runningPromise = testManager.runTest(CommandSource.commandPalette, { testFunction }, false, true);

// This promise should never resolve nor reject.
runningPromise
.then(() => deferred.reject('Debugger stopped when it shouldn\'t have'))
.catch(error => deferred.reject(error));

mockDebugLauncher.launched
.then((launched) => {
if (launched) {
deferred.resolve('');
} else {
deferred.reject('Debugger not launched');
}
}) .catch(error => deferred.reject(error));

await deferred.promise;
}

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

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

testManager.discoverTests(CommandSource.commandPalette, true, true, true);

const discoveryPromise = testManager.discoverTests(CommandSource.commandPalette, true, true, true);
await expect(runningPromise).to.be.rejectedWith(CANCELLATION_REASON, 'Incorrect reason for ending the debugger');
ioc.dispose(); // will cancel test discovery
await expect(discoveryPromise).to.be.rejectedWith(CANCELLATION_REASON, 'Incorrect reason for ending the debugger');
}

test('Debugger should stop when user invokes a test discovery (unittest)', async () => {
Expand Down Expand Up @@ -151,6 +167,7 @@ suite('Unit Tests - debugging', () => {
runningPromise
.then(() => 'Debugger stopped when it shouldn\'t have')
.catch(() => 'Debugger crashed when it shouldn\'t have')
// tslint:disable-next-line: no-floating-promises
.then(error => {
deferred.reject(error);
});
Expand Down
21 changes: 18 additions & 3 deletions src/test/unittests/stoppingDiscoverAndTest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { expect, use } from 'chai';
import * as chaiAsPromised from 'chai-as-promised';
import * as path from 'path';
import { Uri } from 'vscode';
import {createDeferred} from '../../client/common/helpers';
import { Product } from '../../client/common/types';
import { CANCELLATION_REASON, CommandSource, UNITTEST_PROVIDER } from '../../client/unittests/common/constants';
import { ITestDiscoveryService } from '../../client/unittests/common/types';
Expand Down Expand Up @@ -60,9 +61,23 @@ suite('Unit Tests Stopping Discovery and Runner', () => {

const discoveryPromise = mockTestManager.discoverTests(CommandSource.auto);
mockTestManager.discoveryDeferred.resolve(EmptyTests);
mockTestManager.runTest(CommandSource.ui);
const runningPromise = mockTestManager.runTest(CommandSource.ui);
const deferred = createDeferred<string>();

await expect(discoveryPromise).to.eventually.equal(EmptyTests);
// This promise should never resolve nor reject.
runningPromise
.then(() => Promise.reject('Debugger stopped when it shouldn\'t have'))
.catch(error => deferred.reject(error));

discoveryPromise.then(result => {
if (result === EmptyTests) {
deferred.resolve('');
} else {
deferred.reject('tests not empty');
}
}).catch(error => deferred.reject(error));

await deferred.promise;
});

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

// User manually discovering tests will kill the existing test runner.
mockTestManager.discoverTests(CommandSource.ui, true, false, true);
await mockTestManager.discoverTests(CommandSource.ui, true, false, true);
Copy link
Author

@elliott-beach elliott-beach Feb 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This caused the coverage to go down because, without the await, discoverTests would eventually receive a cancellation request, triggering handler code. Edit: that's wrong, it's somewhere else

await expect(runPromise).to.eventually.be.rejectedWith(CANCELLATION_REASON);
});
});