Skip to content

Commit

Permalink
Tweak multi-root support and failed test reruns
Browse files Browse the repository at this point in the history
  • Loading branch information
@karthiknadig
karthiknadig committed Jul 28, 2021
1 parent 3263955 commit 10744e5
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 132 deletions.
25 changes: 10 additions & 15 deletions src/client/testing/testController/common/types.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import { CancellationToken, TestController, TestItem, TestRunRequest, Uri, WorkspaceFolder } from 'vscode';
import { CancellationToken, TestController, TestItem, TestRun, TestRunProfileKind, Uri, WorkspaceFolder } from 'vscode';
import { TestDiscoveryOptions } from '../../common/types';

export type TestRunInstanceOptions = TestRunOptions & {
Expand Down Expand Up @@ -37,28 +37,23 @@ export interface ITestController {
refreshTestData(resource?: Uri, options?: TestRefreshOptions): Promise<void>;
}

export interface ITestRun {
includes: TestItem[];
excludes: TestItem[];
runKind: TestRunProfileKind;
runInstance: TestRun;
}

export const ITestFrameworkController = Symbol('ITestFrameworkController');
export interface ITestFrameworkController {
resolveChildren(testController: TestController, item: TestItem): Promise<void>;
refreshTestData(testController: TestController, resource?: Uri, token?: CancellationToken): Promise<void>;
runTests(
testController: TestController,
request: TestRunRequest,
debug: boolean,
workspace: WorkspaceFolder,
token: CancellationToken,
): Promise<void>;
runTests(testRun: ITestRun, workspace: WorkspaceFolder, token: CancellationToken): Promise<void>;
}

export const ITestsRunner = Symbol('ITestsRunner');
export interface ITestsRunner {
runTests(
testController: TestController,
request: TestRunRequest,
debug: boolean,
options: TestRunOptions,
idToRawData: Map<string, TestData>,
): Promise<void>;
runTests(testRun: ITestRun, options: TestRunOptions, idToRawData: Map<string, TestData>): Promise<void>;
}

export type TestRunOptions = {
Expand Down
109 changes: 89 additions & 20 deletions src/client/testing/testController/controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ import {
RelativePattern,
TestRunProfileKind,
CancellationTokenSource,
Uri,
} from 'vscode';
import { IWorkspaceService } from '../../common/application/types';
import { traceVerbose } from '../../common/logger';
import { IConfigurationService, IDisposableRegistry, Resource } from '../../common/types';
import { DelayedTrigger, IDelayedTrigger } from '../../common/utils/delayTrigger';
import { PYTEST_PROVIDER, UNITTEST_PROVIDER } from '../common/constants';
import { getNodeByUri } from './common/testItemUtilities';
import { ITestController, ITestFrameworkController, TestRefreshOptions } from './common/types';

@injectable()
Expand All @@ -41,7 +43,16 @@ export class PythonTestController implements ITestController {
this.testController = tests.createTestController('python-tests', 'Python Tests');
this.disposables.push(this.testController);

const delayTrigger = new DelayedTrigger(this.refreshTestDataInternal.bind(this), 250, 'Refresh Test Data');
const delayTrigger = new DelayedTrigger(
(uri: Uri, invalidate: boolean) => {
this.refreshTestDataInternal(uri);
if (invalidate) {
this.invalidateTests(uri);
}
},
250, // Delay running the refresh by 250 ms
'Refresh Test Data',
);
this.disposables.push(delayTrigger);
this.refreshData = delayTrigger;

Expand All @@ -68,7 +79,7 @@ export class PythonTestController implements ITestController {
return this.refreshTestDataInternal(uri);
}

this.refreshData.trigger(uri);
this.refreshData.trigger(uri, false);
return Promise.resolve();
}

Expand Down Expand Up @@ -114,19 +125,76 @@ export class PythonTestController implements ITestController {
} else {
(this.workspaceService.workspaceFolders || []).forEach((w) => workspaces.push(w));
}
const debug = request.profile?.kind === TestRunProfileKind.Debug;
await Promise.all(
workspaces.map((workspace) => {
const settings = this.configSettings.getSettings(workspace.uri);
if (settings.testing.pytestEnabled) {
return this.pytest.runTests(this.testController, request, debug, workspace, token);
}
if (settings.testing.unittestEnabled) {
return this.unittest.runTests(this.testController, request, debug, workspace, token);
}
return Promise.resolve();
}),
const runInstance = this.testController.createTestRun(
request,
`Running Tests for Workspace(s): ${workspaces.map((w) => w.uri.fsPath).join(';')}`,
true,
);
const dispose = token.onCancellationRequested(() => {
runInstance.end();
});

try {
await Promise.all(
workspaces.map((workspace) => {
const testItems: TestItem[] = [];
// If the run request includes test items then collect only items that belong to
// `workspace`. If there are no items in the run request then just run the `workspace`
// root test node. Include will be `undefined` in the "run all" scenario.
(request.include ?? this.testController.items).forEach((i: TestItem) => {
const w = this.workspaceService.getWorkspaceFolder(i.uri);
if (w?.uri.fsPath === workspace.uri.fsPath) {
testItems.push(i);
}
});

if (testItems.length > 0) {
const settings = this.configSettings.getSettings(workspace.uri);
if (settings.testing.pytestEnabled) {
return this.pytest.runTests(
{
includes: testItems,
excludes: request.exclude ?? [],
runKind: request.profile?.kind ?? TestRunProfileKind.Run,
runInstance,
},
workspace,
token,
);
}
if (settings.testing.unittestEnabled) {
return this.unittest.runTests(
{
includes: testItems,
excludes: request.exclude ?? [],
runKind: request.profile?.kind ?? TestRunProfileKind.Run,
runInstance,
},
workspace,
token,
);
}
}

return Promise.resolve();
}),
);
} finally {
runInstance.appendOutput(`Finished running tests!\r\n`);
runInstance.end();
dispose.dispose();
}
}

private invalidateTests(uri: Uri) {
this.testController.items.forEach((root) => {
const item = getNodeByUri(root, uri);
if (item && !!item.invalidateResults) {
// Minimize invalidating to test case nodes for the test file where
// the change occurred
item.invalidateResults();
}
});
}

private watchForTestChanges(): void {
Expand All @@ -149,19 +217,19 @@ export class PythonTestController implements ITestController {
this.disposables.push(
watcher.onDidChange((uri) => {
traceVerbose(`Testing: Trigger refresh after change in ${uri.fsPath}`);
this.refreshData.trigger(uri);
this.refreshData.trigger(uri, false);
}),
);
this.disposables.push(
watcher.onDidCreate((uri) => {
traceVerbose(`Testing: Trigger refresh after creating ${uri.fsPath}`);
this.refreshData.trigger(uri);
this.refreshData.trigger(uri, false);
}),
);
this.disposables.push(
watcher.onDidDelete((uri) => {
traceVerbose(`Testing: Trigger refresh after deleting in ${uri.fsPath}`);
this.refreshData.trigger(uri);
this.refreshData.trigger(uri, false);
}),
);
}
Expand All @@ -174,19 +242,20 @@ export class PythonTestController implements ITestController {
this.disposables.push(
watcher.onDidChange((uri) => {
traceVerbose(`Testing: Trigger refresh after change in ${uri.fsPath}`);
this.refreshData.trigger(uri);
// We want to invalidate tests for code change
this.refreshData.trigger(uri, true);
}),
);
this.disposables.push(
watcher.onDidCreate((uri) => {
traceVerbose(`Testing: Trigger refresh after creating ${uri.fsPath}`);
this.refreshData.trigger(uri);
this.refreshData.trigger(uri, false);
}),
);
this.disposables.push(
watcher.onDidDelete((uri) => {
traceVerbose(`Testing: Trigger refresh after deleting in ${uri.fsPath}`);
this.refreshData.trigger(uri);
this.refreshData.trigger(uri, false);
}),
);
}
Expand Down
33 changes: 6 additions & 27 deletions src/client/testing/testController/pytest/pytestController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { inject, injectable, named } from 'inversify';
import { flatten } from 'lodash';
import * as path from 'path';
import { CancellationToken, TestItem, TestRunRequest, Uri, TestController, WorkspaceFolder } from 'vscode';
import { CancellationToken, TestItem, Uri, TestController, WorkspaceFolder } from 'vscode';
import { IWorkspaceService } from '../../../common/application/types';
import { traceError } from '../../../common/logger';
import { runAdapter } from '../../../common/process/internal/scripts/testing_tools';
Expand All @@ -25,6 +25,7 @@ import {
ITestsRunner,
TestData,
RawDiscoveredTests,
ITestRun,
} from '../common/types';
import { preparePytestArgumentsForDiscovery, pytestGetTestFolders } from './arguments';

Expand Down Expand Up @@ -79,8 +80,8 @@ export class PytestController implements ITestFrameworkController {
item.description = workspace.uri.fsPath;

// To figure out which top level nodes have to removed. First we get all the
// existing nodes. Then if they have data we keep thoese nodes, Nodes with
// node data will be removed after we check the raw data.
// existing nodes. Then if they have data we keep those nodes, Nodes without
// data will be removed after we check the raw data.
let subRootWithNoData: string[] = [];
item.children.forEach((c) => subRootWithNoData.push(c.id));

Expand Down Expand Up @@ -232,32 +233,10 @@ export class PytestController implements ITestFrameworkController {
return Promise.resolve();
}

public runTests(
testController: TestController,
request: TestRunRequest,
debug: boolean,
workspace: WorkspaceFolder,
token: CancellationToken,
): Promise<void> {
let runRequest = request;
if (!runRequest.include) {
const testItems: TestItem[] = [];
testController.items.forEach((i) => {
const w = this.workspaceService.getWorkspaceFolder(i.uri);
if (w?.uri.fsPath === workspace.uri.fsPath) {
testItems.push(i);
}
});
if (testItems.length > 0) {
runRequest = new TestRunRequest(testItems, undefined, request.profile);
}
}

public runTests(testRun: ITestRun, workspace: WorkspaceFolder, token: CancellationToken): Promise<void> {
const settings = this.configService.getSettings(workspace.uri);
return this.runner.runTests(
testController,
runRequest,
debug,
testRun,
{
workspaceFolder: workspace.uri,
cwd: settings.testing.cwd ?? workspace.uri.fsPath,
Expand Down
29 changes: 11 additions & 18 deletions src/client/testing/testController/pytest/runner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Licensed under the MIT License.

import { inject, injectable, named } from 'inversify';
import { Disposable, TestController, TestItem, TestRun, TestRunRequest } from 'vscode';
import { Disposable, TestItem, TestRun, TestRunProfileKind } from 'vscode';
import { IOutputChannel } from '../../../common/types';
import { PYTEST_PROVIDER } from '../../common/constants';
import { ITestDebugLauncher, ITestRunner, LaunchOptions, Options } from '../../common/types';
Expand All @@ -11,7 +11,7 @@ import { filterArguments, getOptionValues } from '../common/argumentsHelper';
import { createTemporaryFile } from '../common/externalDependencies';
import { updateResultFromJunitXml } from '../common/resultsHelper';
import { getTestCaseNodes } from '../common/testItemUtilities';
import { ITestsRunner, TestData, TestRunInstanceOptions, TestRunOptions } from '../common/types';
import { ITestRun, ITestsRunner, TestData, TestRunInstanceOptions, TestRunOptions } from '../common/types';
import { removePositionalFoldersAndFiles } from './arguments';

const JunitXmlArgOld = '--junitxml';
Expand All @@ -36,31 +36,24 @@ export class PytestRunner implements ITestsRunner {
) {}

public async runTests(
testController: TestController,
request: TestRunRequest,
debug: boolean,
testRun: ITestRun,
options: TestRunOptions,
idToRawData: Map<string, TestData>,
): Promise<void> {
const runOptions: TestRunInstanceOptions = {
...options,
exclude: request.exclude,
debug,
exclude: testRun.excludes,
debug: testRun.runKind === TestRunProfileKind.Debug,
};
const runInstance = testController.createTestRun(
request,
`Running Tests for Workspace ${runOptions.workspaceFolder.fsPath}`,
true,
);

try {
await Promise.all(
(request.include ?? []).map((testNode) => this.runTest(testNode, runInstance, runOptions, idToRawData)),
testRun.includes.map((testNode) =>
this.runTest(testNode, testRun.runInstance, runOptions, idToRawData),
),
);
} catch (ex) {
runInstance.appendOutput(`Error while running tests:\r\n${ex}\r\n\r\n`);
} finally {
runInstance.appendOutput(`Finished running tests!\r\n`);
runInstance.end();
testRun.runInstance.appendOutput(`Error while running tests:\r\n${ex}\r\n\r\n`);
}
}

Expand All @@ -70,7 +63,7 @@ export class PytestRunner implements ITestsRunner {
options: TestRunInstanceOptions,
idToRawData: Map<string, TestData>,
): Promise<void> {
runInstance.appendOutput(`Running tests: ${testNode.label}\r\n`);
runInstance.appendOutput(`Running tests (pytest): ${testNode.id}\r\n`);

// VS Code API requires that we set the run state on the leaf nodes. The state of the
// parent nodes are computed based on the state of child nodes.
Expand Down
Loading

0 comments on commit 10744e5

Please sign in to comment.