Skip to content

Commit 692365e

Browse files
SalmonModeDonJayamanne
SalmonMode
authored and
DonJayamanne
committed
Show diagnostics for failed/skipped tests run with pytest (microsoft#3303)
* connect pytest test run output to problems pane * added localization capabilities for pytest diagnostic message prefixes * added unit tests for unittest diagnostic service * updated test plan * adding news file * fixed floating promise * fixed typos in localization keys * compensation for windows vs mac file path differences * more fixes for windows vs mac file path compatibility * fixed regex to account for windows, added non-null assertion, and removed commented out code * fixed issues with windows file paths (Uri.file().fsPath alters the case of the drive letter) * added TestMessageService, fixed issue with new lines in diagnostic messages, fixed issue with diagnostics not going away due to their name changing, and cleaned up code. * Fixed promises and linter issues
1 parent 83f38ec commit 692365e

28 files changed

Lines changed: 1882 additions & 333 deletions

‎.github/test_plan.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,10 @@ def test_failure():
288288
- [ ] Tests are discovered (as shown by code lenses on each test)
289289
- [ ] `Run Test` works
290290
- [ ] `Debug Test` works
291+
- [ ] A `Diagnostic` is shown in the problems pane for each failed/skipped test
292+
- [ ] The `Diagnostic`s are organized according to the file the test was executed from (not neccesarily the file it was defined in)
293+
- [ ] The appropriate `DiagnosticRelatedInformation` is shown for each `Diagnostic`
294+
- [ ] The `DiagnosticRelatedInformation` reflects the traceback for the test
291295

292296
#### [`nose`](https://code.visualstudio.com/docs/python/unit-testing#_nose-configuration-settings)
293297
```python

‎news/1 Enhancements/120.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Create diagnostics for failed/skipped tests that were run with pytest.
2+
(thanks [Chris NeJame](https://github.com/SalmonMode/))

‎package.nls.json‎

Lines changed: 179 additions & 176 deletions
Large diffs are not rendered by default.

‎src/client/common/utils/localize.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,12 @@ export namespace DebugConfigurationPrompts {
144144
export const attachRemoteHostValidationError = localize('debug.attachRemoteHostValidationError', 'Enter a host name or IP address');
145145
}
146146

147+
export namespace UnitTests {
148+
export const testErrorDiagnosticMessage = localize('UnitTests.testErrorDiagnosticMessage', 'Error');
149+
export const testFailDiagnosticMessage = localize('UnitTests.testFailDiagnosticMessage', 'Fail');
150+
export const testSkippedDiagnosticMessage = localize('UnitTests.testSkippedDiagnosticMessage', 'Skipped');
151+
}
152+
147153
// Skip using vscode-nls and instead just compute our strings based on key values. Key values
148154
// can be loaded out of the nls.<locale>.json files
149155
let loadedCollection: { [index: string]: string } | undefined;

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

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
1-
import { CancellationToken, CancellationTokenSource, Disposable, OutputChannel, Uri } from 'vscode';
1+
import { CancellationToken, CancellationTokenSource, Diagnostic, DiagnosticCollection, DiagnosticRelatedInformation, Disposable, languages, OutputChannel, Uri } from 'vscode';
22
import { IWorkspaceService } from '../../../common/application/types';
33
import { isNotInstalledError } from '../../../common/helpers';
4+
import { IFileSystem } from '../../../common/platform/types';
45
import { IConfigurationService, IDisposableRegistry, IInstaller, IOutputChannel, IPythonSettings, Product } from '../../../common/types';
56
import { getNamesAndValues } from '../../../common/utils/enum';
67
import { IServiceContainer } from '../../../ioc/types';
78
import { UNITTEST_DISCOVER, UNITTEST_RUN } from '../../../telemetry/constants';
89
import { sendTelemetryEvent } from '../../../telemetry/index';
910
import { TestDiscoverytTelemetry, TestRunTelemetry } from '../../../telemetry/types';
11+
import { IPythonUnitTestMessage, IUnitTestDiagnosticService } from '../../types';
1012
import { CANCELLATION_REASON, CommandSource, TEST_OUTPUT_CHANNEL } from './../constants';
1113
import { ITestCollectionStorageService, ITestDiscoveryService, ITestManager, ITestResultsService, ITestsHelper, TestDiscoveryOptions, TestProvider, Tests, TestStatus, TestsToRun } from './../types';
1214

@@ -16,7 +18,9 @@ enum CancellationTokenType {
1618
}
1719

1820
export abstract class BaseTestManager implements ITestManager {
21+
public diagnosticCollection: DiagnosticCollection;
1922
protected readonly settings: IPythonSettings;
23+
private readonly unitTestDiagnosticService: IUnitTestDiagnosticService;
2024
public abstract get enabled(): boolean;
2125
protected get outputChannel() {
2226
return this._outputChannel;
@@ -50,6 +54,8 @@ export abstract class BaseTestManager implements ITestManager {
5054
this.testCollectionStorage = this.serviceContainer.get<ITestCollectionStorageService>(ITestCollectionStorageService);
5155
this._testResultsService = this.serviceContainer.get<ITestResultsService>(ITestResultsService);
5256
this.workspaceService = this.serviceContainer.get<IWorkspaceService>(IWorkspaceService);
57+
this.diagnosticCollection = languages.createDiagnosticCollection(this.testProvider);
58+
this.unitTestDiagnosticService = serviceContainer.get<IUnitTestDiagnosticService>(IUnitTestDiagnosticService);
5359
disposables.push(this);
5460
}
5561
protected get testDiscoveryCancellationToken(): CancellationToken | undefined {
@@ -246,6 +252,42 @@ export abstract class BaseTestManager implements ITestManager {
246252
return Promise.reject<Tests>(reason);
247253
});
248254
}
255+
public async updateDiagnostics(tests: Tests, messages: IPythonUnitTestMessage[]): Promise<void> {
256+
await this.stripStaleDiagnostics(tests, messages);
257+
258+
// Update relevant file diagnostics for tests that have problems.
259+
const uniqueMsgFiles = messages.reduce((filtered, msg) => {
260+
if (filtered.indexOf(msg.testFilePath) === -1 && msg.testFilePath !== undefined) {
261+
filtered.push(msg.testFilePath);
262+
}
263+
return filtered;
264+
}, []);
265+
const fs = this.serviceContainer.get<IFileSystem>(IFileSystem);
266+
for (const msgFile of uniqueMsgFiles) {
267+
// Check all messages against each test file.
268+
const fileUri = Uri.file(msgFile);
269+
if (!this.diagnosticCollection.has(fileUri)) {
270+
// Create empty diagnostic for file URI so the rest of the logic can assume one already exists.
271+
const diagnostics: Diagnostic[] = [];
272+
this.diagnosticCollection.set(fileUri, diagnostics);
273+
}
274+
// Get the diagnostics for this file's URI before updating it so old tests that weren't run can still show problems.
275+
const oldDiagnostics = this.diagnosticCollection.get(fileUri);
276+
const newDiagnostics: Diagnostic[] = [];
277+
for (const diagnostic of oldDiagnostics) {
278+
newDiagnostics.push(diagnostic);
279+
}
280+
for (const msg of messages) {
281+
if (fs.arePathsSame(fileUri.fsPath, Uri.file(msg.testFilePath).fsPath) && msg.status !== TestStatus.Pass) {
282+
const diagnostic = this.createDiagnostics(msg);
283+
newDiagnostics.push(diagnostic);
284+
}
285+
}
286+
287+
// Set the diagnostics for the file.
288+
this.diagnosticCollection.set(fileUri, newDiagnostics);
289+
}
290+
}
249291
// tslint:disable-next-line:no-any
250292
protected abstract runTestImpl(tests: Tests, testsToRun?: TestsToRun, runFailedTests?: boolean, debug?: boolean): Promise<any>;
251293
protected abstract getDiscoveryOptions(ignoreCache: boolean): TestDiscoveryOptions;
@@ -270,4 +312,51 @@ export abstract class BaseTestManager implements ITestManager {
270312
this.testRunnerCancellationTokenSource = undefined;
271313
}
272314
}
315+
/**
316+
* Whenever a test is run, any previous problems it had should be removed. This runs through
317+
* every already existing set of diagnostics for any that match the tests that were just run
318+
* so they can be stripped out (as they are now no longer relevant). If the tests pass, then
319+
* there is no need to have a diagnostic for it. If they fail, the stale diagnostic will be
320+
* replaced by an up-to-date diagnostic showing the most recent problem with that test.
321+
*
322+
* In order to identify diagnostics associated with the tests that were run, the `nameToRun`
323+
* property of each messages is compared to the `code` property of each diagnostic.
324+
*
325+
* @param messages Details about the tests that were just run.
326+
*/
327+
private async stripStaleDiagnostics(tests: Tests, messages: IPythonUnitTestMessage[]): Promise<void> {
328+
this.diagnosticCollection.forEach((diagnosticUri, oldDiagnostics, collection) => {
329+
const newDiagnostics: Diagnostic[] = [];
330+
for (const diagnostic of oldDiagnostics) {
331+
const matchingMsg = messages.find((msg) => msg.code === diagnostic.code);
332+
if (matchingMsg === undefined) {
333+
// No matching message was found, so this test was not included in the test run.
334+
const matchingTest = tests.testFunctions.find((tf) => tf.testFunction.nameToRun === diagnostic.code);
335+
if (matchingTest !== undefined) {
336+
// Matching test was found, so the diagnostic is still relevant.
337+
newDiagnostics.push(diagnostic);
338+
}
339+
}
340+
}
341+
// Set the diagnostics for the file.
342+
collection.set(diagnosticUri, newDiagnostics);
343+
});
344+
}
345+
346+
private createDiagnostics(message: IPythonUnitTestMessage): Diagnostic {
347+
const stackStart = message.locationStack[0];
348+
const diagPrefix = this.unitTestDiagnosticService.getMessagePrefix(message.status);
349+
const severity = this.unitTestDiagnosticService.getSeverity(message.severity)!;
350+
const diagMsg = message.message.split('\n')[0];
351+
const diagnostic = new Diagnostic(stackStart.location.range, `${diagPrefix ? `${diagPrefix}: ` : ''}${diagMsg}`, severity);
352+
diagnostic.code = message.code;
353+
diagnostic.source = message.provider;
354+
const relatedInfoArr: DiagnosticRelatedInformation[] = [];
355+
for (const frameDetails of message.locationStack) {
356+
const relatedInfo = new DiagnosticRelatedInformation(frameDetails.location, frameDetails.lineText);
357+
relatedInfoArr.push(relatedInfo);
358+
}
359+
diagnostic.relatedInformation = relatedInfoArr;
360+
return diagnostic;
361+
}
273362
}

‎src/client/unittests/common/services/unitTestDiagnosticService.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
6+
import { injectable } from 'inversify';
7+
import { DiagnosticSeverity } from 'vscode';
8+
import * as localize from '../../../common/utils/localize';
9+
import { DiagnosticMessageType, IUnitTestDiagnosticService, PythonUnitTestMessageSeverity } from '../../types';
10+
import { TestStatus } from '../types';
11+
12+
@injectable()
13+
export class UnitTestDiagnosticService implements IUnitTestDiagnosticService {
14+
private MessageTypes = new Map<TestStatus, DiagnosticMessageType>();
15+
private MessageSeverities = new Map<PythonUnitTestMessageSeverity, DiagnosticSeverity>();
16+
private MessagePrefixes = new Map<DiagnosticMessageType, string>();
17+
18+
constructor() {
19+
this.MessageTypes.set(TestStatus.Error, DiagnosticMessageType.Error);
20+
this.MessageTypes.set(TestStatus.Fail, DiagnosticMessageType.Fail);
21+
this.MessageTypes.set(TestStatus.Skipped, DiagnosticMessageType.Skipped);
22+
this.MessageTypes.set(TestStatus.Pass, DiagnosticMessageType.Pass);
23+
this.MessageSeverities.set(PythonUnitTestMessageSeverity.Error, DiagnosticSeverity.Error);
24+
this.MessageSeverities.set(PythonUnitTestMessageSeverity.Failure, DiagnosticSeverity.Error);
25+
this.MessageSeverities.set(PythonUnitTestMessageSeverity.Skip, DiagnosticSeverity.Information);
26+
this.MessageSeverities.set(PythonUnitTestMessageSeverity.Pass, null);
27+
this.MessagePrefixes.set(DiagnosticMessageType.Error, localize.UnitTests.testErrorDiagnosticMessage());
28+
this.MessagePrefixes.set(DiagnosticMessageType.Fail, localize.UnitTests.testFailDiagnosticMessage());
29+
this.MessagePrefixes.set(DiagnosticMessageType.Skipped, localize.UnitTests.testSkippedDiagnosticMessage());
30+
this.MessagePrefixes.set(DiagnosticMessageType.Pass, '');
31+
}
32+
public getMessagePrefix(status: TestStatus): string {
33+
return this.MessagePrefixes.get(this.MessageTypes.get(status));
34+
}
35+
public getSeverity(unitTestSeverity: PythonUnitTestMessageSeverity): DiagnosticSeverity {
36+
return this.MessageSeverities.get(unitTestSeverity);
37+
}
38+
}

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { CancellationToken, Disposable, OutputChannel, Uri } from 'vscode';
1+
import { CancellationToken, DiagnosticCollection, Disposable, OutputChannel, Uri } from 'vscode';
22
import { IUnitTestSettings, Product } from '../../common/types';
3+
import { IPythonUnitTestMessage } from '../types';
34
import { CommandSource } from './constants';
45

56
export type TestProvider = 'nosetest' | 'pytest' | 'unittest';
@@ -66,6 +67,7 @@ export type TestResult = Node & {
6667
passed?: boolean;
6768
time: number;
6869
line?: number;
70+
file?: string;
6971
message?: string;
7072
traceback?: string;
7173
functionsPassed?: number;
@@ -223,6 +225,7 @@ export interface ITestManager extends Disposable {
223225
readonly enabled: boolean;
224226
readonly workingDirectory: string;
225227
readonly workspaceFolder: Uri;
228+
diagnosticCollection: DiagnosticCollection;
226229
stop(): void;
227230
resetTestResults(): void;
228231
discoverTests(cmdSource: CommandSource, ignoreCache?: boolean, quietMode?: boolean, userInitiated?: boolean): Promise<Tests>;
@@ -278,3 +281,8 @@ export type PythonVersionInformation = {
278281
major: number;
279282
minor: number;
280283
};
284+
285+
export const ITestMessageService = Symbol('ITestMessageService');
286+
export interface ITestMessageService {
287+
getFilteredTestMessages(rootDirectory: string, testResults: Tests): Promise<IPythonUnitTestMessage[]>;
288+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ export function updateResultsFromXmlLogFile(tests: Tests, outputXmlFile: string,
110110
}
111111

112112
result.testFunction.line = getSafeInt(testcase.$.line, null);
113+
result.testFunction.file = testcase.$.file;
113114
result.testFunction.time = parseFloat(testcase.$.time);
114115
result.testFunction.passed = true;
115116
result.testFunction.status = TestStatus.Pass;

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@ import { Product } from '../../common/types';
55
import { IServiceContainer } from '../../ioc/types';
66
import { PYTEST_PROVIDER } from '../common/constants';
77
import { BaseTestManager } from '../common/managers/baseTestManager';
8-
import { ITestsHelper, TestDiscoveryOptions, TestRunOptions, Tests, TestsToRun } from '../common/types';
9-
import { IArgumentsService, ITestManagerRunner, TestFilter } from '../types';
8+
import { ITestMessageService, ITestsHelper, TestDiscoveryOptions, TestRunOptions, Tests, TestsToRun } from '../common/types';
9+
import { IArgumentsService, IPythonUnitTestMessage, ITestManagerRunner, TestFilter } from '../types';
1010

1111
export class TestManager extends BaseTestManager {
1212
private readonly argsService: IArgumentsService;
1313
private readonly helper: ITestsHelper;
1414
private readonly runner: ITestManagerRunner;
15+
private readonly testMessageService: ITestMessageService;
1516
public get enabled() {
1617
return this.settings.unitTest.pyTestEnabled;
1718
}
@@ -21,6 +22,7 @@ export class TestManager extends BaseTestManager {
2122
this.argsService = this.serviceContainer.get<IArgumentsService>(IArgumentsService, this.testProvider);
2223
this.helper = this.serviceContainer.get<ITestsHelper>(ITestsHelper);
2324
this.runner = this.serviceContainer.get<ITestManagerRunner>(ITestManagerRunner, this.testProvider);
25+
this.testMessageService = this.serviceContainer.get<ITestMessageService>(ITestMessageService, this.testProvider);
2426
}
2527
public getDiscoveryOptions(ignoreCache: boolean): TestDiscoveryOptions {
2628
const args = this.settings.unitTest.pyTestArgs.slice(0);
@@ -51,6 +53,9 @@ export class TestManager extends BaseTestManager {
5153
token: this.testRunnerCancellationToken!,
5254
outChannel: this.outputChannel
5355
};
54-
return this.runner.runTest(this.testResultsService, options, this);
56+
const testResults = await this.runner.runTest(this.testResultsService, options, this);
57+
const messages: IPythonUnitTestMessage[] = await this.testMessageService.getFilteredTestMessages(this.rootDirectory, testResults);
58+
await this.updateDiagnostics(tests, messages);
59+
return testResults;
5560
}
5661
}

0 commit comments

Comments
 (0)