diff --git a/news/2 Fixes/6303.md b/news/2 Fixes/6303.md new file mode 100644 index 000000000000..1926ad186b2c --- /dev/null +++ b/news/2 Fixes/6303.md @@ -0,0 +1 @@ +Fix code lenses shown for pytest diff --git a/src/client/testing/codeLenses/main.ts b/src/client/testing/codeLenses/main.ts index 9f955d485f34..7b54d0ffe00c 100644 --- a/src/client/testing/codeLenses/main.ts +++ b/src/client/testing/codeLenses/main.ts @@ -1,4 +1,5 @@ import * as vscode from 'vscode'; +import { IServiceContainer } from '../../../client/ioc/types'; import { PYTHON } from '../../common/constants'; import { ITestCollectionStorageService } from '../common/types'; import { TestFileCodeLensProvider } from './testFiles'; @@ -6,10 +7,11 @@ import { TestFileCodeLensProvider } from './testFiles'; export function activateCodeLenses( onDidChange: vscode.EventEmitter, symbolProvider: vscode.DocumentSymbolProvider, - testCollectionStorage: ITestCollectionStorageService + testCollectionStorage: ITestCollectionStorageService, + serviceContainer: IServiceContainer ): vscode.Disposable { const disposables: vscode.Disposable[] = []; - const codeLensProvider = new TestFileCodeLensProvider(onDidChange, symbolProvider, testCollectionStorage); + const codeLensProvider = new TestFileCodeLensProvider(onDidChange, symbolProvider, testCollectionStorage, serviceContainer); disposables.push(vscode.languages.registerCodeLensProvider(PYTHON, codeLensProvider)); return { diff --git a/src/client/testing/codeLenses/testFiles.ts b/src/client/testing/codeLenses/testFiles.ts index 140d1bddb702..19cf9a5c1783 100644 --- a/src/client/testing/codeLenses/testFiles.ts +++ b/src/client/testing/codeLenses/testFiles.ts @@ -2,7 +2,10 @@ // tslint:disable:no-object-literal-type-assertion -import { CancellationToken, CancellationTokenSource, CodeLens, CodeLensProvider, DocumentSymbolProvider, Event, EventEmitter, Position, Range, SymbolInformation, SymbolKind, TextDocument, Uri, workspace } from 'vscode'; +import { CancellationToken, CancellationTokenSource, CodeLens, CodeLensProvider, DocumentSymbolProvider, Event, EventEmitter, Position, Range, SymbolInformation, SymbolKind, TextDocument, Uri } from 'vscode'; +import { IWorkspaceService } from '../../../client/common/application/types'; +import { IFileSystem } from '../../../client/common/platform/types'; +import { IServiceContainer } from '../../../client/ioc/types'; import * as constants from '../../common/constants'; import { CommandSource } from '../common/constants'; import { ITestCollectionStorageService, TestFile, TestFunction, TestStatus, TestsToRun, TestSuite } from '../common/types'; @@ -13,10 +16,15 @@ type FunctionsAndSuites = { }; export class TestFileCodeLensProvider implements CodeLensProvider { + private workspaceService: IWorkspaceService; + private fileSystem: IFileSystem; // tslint:disable-next-line:variable-name constructor(private _onDidChange: EventEmitter, private symbolProvider: DocumentSymbolProvider, - private testCollectionStorage: ITestCollectionStorageService) { + private testCollectionStorage: ITestCollectionStorageService, + serviceContainer: IServiceContainer) { + this.workspaceService = serviceContainer.get(IWorkspaceService); + this.fileSystem = serviceContainer.get(IFileSystem); } get onDidChangeCodeLenses(): Event { @@ -24,7 +32,7 @@ export class TestFileCodeLensProvider implements CodeLensProvider { } public async provideCodeLenses(document: TextDocument, token: CancellationToken) { - const wkspace = workspace.getWorkspaceFolder(document.uri); + const wkspace = this.workspaceService.getWorkspaceFolder(document.uri); if (!wkspace) { return []; } @@ -52,16 +60,20 @@ export class TestFileCodeLensProvider implements CodeLensProvider { return Promise.resolve(codeLens); } - private async getCodeLenses(document: TextDocument, token: CancellationToken, symbolProvider: DocumentSymbolProvider) { - const wkspace = workspace.getWorkspaceFolder(document.uri); + public getTestFileWhichNeedsCodeLens(document: TextDocument): TestFile | undefined { + const wkspace = this.workspaceService.getWorkspaceFolder(document.uri); if (!wkspace) { - return []; + return; } const tests = this.testCollectionStorage.getTests(wkspace.uri); if (!tests) { - return []; + return; } - const file = tests.testFiles.find(item => item.fullPath === document.uri.fsPath); + return tests.testFiles.find(item => this.fileSystem.arePathsSame(item.fullPath, document.uri.fsPath)); + } + + private async getCodeLenses(document: TextDocument, token: CancellationToken, symbolProvider: DocumentSymbolProvider) { + const file = this.getTestFileWhichNeedsCodeLens(document); if (!file) { return []; } diff --git a/src/client/testing/common/services/discoveredTestParser.ts b/src/client/testing/common/services/discoveredTestParser.ts index ed123270b12e..7772a35fc894 100644 --- a/src/client/testing/common/services/discoveredTestParser.ts +++ b/src/client/testing/common/services/discoveredTestParser.ts @@ -58,7 +58,7 @@ export class TestDiscoveredTestParser implements ITestDiscoveredTestParser { * @param {Tests} tests * @memberof TestsDiscovery */ - protected buildChildren(rootFolder: TestFolder, parent: TestDataItem, discoveredTests: DiscoveredTests, tests: Tests) { + public buildChildren(rootFolder: TestFolder, parent: TestDataItem, discoveredTests: DiscoveredTests, tests: Tests) { const parentType = getTestType(parent); switch (parentType) { case TestType.testFolder: { diff --git a/src/client/testing/main.ts b/src/client/testing/main.ts index fc71ecaa22a1..b9b697391b80 100644 --- a/src/client/testing/main.ts +++ b/src/client/testing/main.ts @@ -324,7 +324,7 @@ export class UnitTestManagementService implements ITestManagementService, Dispos } }); this.disposableRegistry.push(handler); - this.disposableRegistry.push(activateCodeLenses(event, symbolProvider, testCollectionStorage)); + this.disposableRegistry.push(activateCodeLenses(event, symbolProvider, testCollectionStorage, this.serviceContainer)); } @captureTelemetry(EventName.UNITTEST_CONFIGURE, undefined, false) diff --git a/src/test/testing/codeLenses/testFiles.unit.test.ts b/src/test/testing/codeLenses/testFiles.unit.test.ts new file mode 100644 index 000000000000..ce262c9d3d4b --- /dev/null +++ b/src/test/testing/codeLenses/testFiles.unit.test.ts @@ -0,0 +1,158 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +// tslint:disable:no-any + +import { assert, expect } from 'chai'; +import { mock } from 'ts-mockito'; +import * as typemoq from 'typemoq'; +import { DocumentSymbolProvider, EventEmitter, Uri } from 'vscode'; +import { IWorkspaceService } from '../../../client/common/application/types'; +import { IFileSystem } from '../../../client/common/platform/types'; +import { IServiceContainer } from '../../../client/ioc/types'; +import { LanguageServerSymbolProvider } from '../../../client/providers/symbolProvider'; +import { TestFileCodeLensProvider } from '../../../client/testing/codeLenses/testFiles'; +import { ITestCollectionStorageService } from '../../../client/testing/common/types'; + +// tslint:disable-next-line: max-func-body-length +suite('Code lenses - Test files', () => { + let testCollectionStorage: typemoq.IMock; + let workspaceService: typemoq.IMock; + let fileSystem: typemoq.IMock; + let serviceContainer: typemoq.IMock; + let symbolProvider: DocumentSymbolProvider; + let onDidChange: EventEmitter; + let codeLensProvider: TestFileCodeLensProvider; + setup(() => { + workspaceService = typemoq.Mock.ofType(); + fileSystem = typemoq.Mock.ofType(); + testCollectionStorage = typemoq.Mock.ofType(); + serviceContainer = typemoq.Mock.ofType(); + symbolProvider = mock(LanguageServerSymbolProvider); + onDidChange = new EventEmitter(); + serviceContainer + .setup(c => c.get(typemoq.It.isValue(IWorkspaceService))) + .returns(() => workspaceService.object); + serviceContainer + .setup(c => c.get(typemoq.It.isValue(IFileSystem))) + .returns(() => fileSystem.object); + codeLensProvider = new TestFileCodeLensProvider(onDidChange, symbolProvider, testCollectionStorage.object, serviceContainer.object); + }); + + teardown(() => { + onDidChange.dispose(); + }); + + test('Function getTestFileWhichNeedsCodeLens() returns `undefined` if there are no workspace corresponding to document', async () => { + const document = { + uri: Uri.file('path/to/document') + }; + workspaceService + .setup(w => w.getWorkspaceFolder(document.uri)) + .returns(() => undefined) + .verifiable(typemoq.Times.once()); + testCollectionStorage + .setup(w => w.getTests(typemoq.It.isAny())) + .returns(() => undefined) + .verifiable(typemoq.Times.never()); + const files = codeLensProvider.getTestFileWhichNeedsCodeLens(document as any); + expect(files).to.equal(undefined, 'No files should be returned'); + workspaceService.verifyAll(); + testCollectionStorage.verifyAll(); + }); + + test('Function getTestFileWhichNeedsCodeLens() returns `undefined` if test storage is empty', async () => { + const document = { + uri: Uri.file('path/to/document') + }; + const workspaceUri = Uri.file('path/to/workspace'); + const workspace = { uri: workspaceUri }; + workspaceService + .setup(w => w.getWorkspaceFolder(document.uri)) + .returns(() => workspace as any) + .verifiable(typemoq.Times.once()); + testCollectionStorage + .setup(w => w.getTests(workspaceUri)) + .returns(() => undefined) + .verifiable(typemoq.Times.once()); + const files = codeLensProvider.getTestFileWhichNeedsCodeLens(document as any); + expect(files).to.equal(undefined, 'No files should be returned'); + workspaceService.verifyAll(); + testCollectionStorage.verifyAll(); + }); + + test('Function getTestFileWhichNeedsCodeLens() returns `undefined` if tests returned from storage does not contain document', async () => { + const document = { + uri: Uri.file('path/to/document5') + }; + const workspaceUri = Uri.file('path/to/workspace'); + const workspace = { uri: workspaceUri }; + const tests = { + testFiles: [ + { + fullPath: 'path/to/document1' + }, + { + fullPath: 'path/to/document2' + } + ] + }; + workspaceService + .setup(w => w.getWorkspaceFolder(document.uri)) + .returns(() => workspace as any) + .verifiable(typemoq.Times.once()); + testCollectionStorage + .setup(w => w.getTests(workspaceUri)) + .returns(() => tests as any) + .verifiable(typemoq.Times.once()); + fileSystem + .setup(f => f.arePathsSame('path/to/document1', 'path/to/document5')) + .returns(() => false); + fileSystem + .setup(f => f.arePathsSame('path/to/document2', 'path/to/document5')) + .returns(() => false); + const files = codeLensProvider.getTestFileWhichNeedsCodeLens(document as any); + expect(files).to.equal(undefined, 'No files should be returned'); + workspaceService.verifyAll(); + testCollectionStorage.verifyAll(); + }); + + test('Function getTestFileWhichNeedsCodeLens() returns test file if tests returned from storage contains document', async () => { + const document = { + uri: Uri.file('path/to/document2') + }; + const workspaceUri = Uri.file('path/to/workspace'); + const workspace = { uri: workspaceUri }; + const testFile2 = { + fullPath: 'path/to/document2' + }; + const tests = { + testFiles: [ + { + fullPath: 'path/to/document1' + }, + testFile2 + ] + }; + workspaceService + .setup(w => w.getWorkspaceFolder(document.uri)) + .returns(() => workspace as any) + .verifiable(typemoq.Times.once()); + testCollectionStorage + .setup(w => w.getTests(workspaceUri)) + .returns(() => tests as any) + .verifiable(typemoq.Times.once()); + fileSystem + .setup(f => f.arePathsSame('path/to/document1', 'path/to/document2')) + .returns(() => false); + fileSystem + .setup(f => f.arePathsSame('path/to/document2', 'path/to/document2')) + .returns(() => true); + const files = codeLensProvider.getTestFileWhichNeedsCodeLens(document as any); + assert.deepEqual(files, testFile2 as any); + workspaceService.verifyAll(); + testCollectionStorage.verifyAll(); + }); +}); diff --git a/src/test/testing/common/services/discoveredTestParser.unit.test.ts b/src/test/testing/common/services/discoveredTestParser.unit.test.ts new file mode 100644 index 000000000000..b1e379c7bd9d --- /dev/null +++ b/src/test/testing/common/services/discoveredTestParser.unit.test.ts @@ -0,0 +1,102 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import * as typemoq from 'typemoq'; +import { Uri } from 'vscode'; +import { IWorkspaceService } from '../../../../client/common/application/types'; +import { TestDiscoveredTestParser } from '../../../../client/testing/common/services/discoveredTestParser'; +import { Tests } from '../../../../client/testing/common/types'; + +// tslint:disable:no-any max-func-body-length +suite('Services - Discovered test parser', () => { + let workspaceService: typemoq.IMock; + let parser: TestDiscoveredTestParser; + setup(() => { + workspaceService = typemoq.Mock.ofType(); + }); + + teardown(() => { + sinon.restore(); + }); + + test('Parse returns empty tests if resource does not belong to workspace', () => { + // That is, getWorkspaceFolder() returns undefined. + const expectedTests: Tests = { + rootTestFolders: [], + summary: { errors: 0, failures: 0, passed: 0, skipped: 0 }, + testFiles: [], + testFolders: [], + testFunctions: [], + testSuites: [] + }; + const discoveredTests = [{ + root: 'path/to/testDataRoot' + }]; + const buildChildren = sinon.stub(TestDiscoveredTestParser.prototype, 'buildChildren'); + buildChildren.callsFake(() => undefined); + workspaceService + .setup(w => w.getWorkspaceFolder(typemoq.It.isAny())) + .returns(() => undefined) + .verifiable(typemoq.Times.once()); + parser = new TestDiscoveredTestParser(workspaceService.object); + const result = parser.parse(Uri.file('path/to/resource'), discoveredTests as any); + assert.ok(buildChildren.notCalled); + assert.deepEqual(expectedTests, result); + workspaceService.verifyAll(); + }); + + test('Parse returns expected tests otherwise', () => { + const discoveredTests = [ + { + root: 'path/to/testDataRoot1', + rootid: 'rootId1' + }, + { + root: 'path/to/testDataRoot2', + rootid: 'rootId2' + } + ]; + const workspaceUri = Uri.file('path/to/workspace'); + const workspace = { uri: workspaceUri }; + const expectedTests: Tests = { + rootTestFolders: [ + { + name: 'path/to/testDataRoot1', folders: [], time: 0, + testFiles: [], resource: workspaceUri, nameToRun: 'rootId1' + }, + { + name: 'path/to/testDataRoot2', folders: [], time: 0, + testFiles: [], resource: workspaceUri, nameToRun: 'rootId2' + } + ], + summary: { errors: 0, failures: 0, passed: 0, skipped: 0 }, + testFiles: [], + testFolders: [ + { + name: 'path/to/testDataRoot1', folders: [], time: 0, + testFiles: [], resource: workspaceUri, nameToRun: 'rootId1' + }, + { + name: 'path/to/testDataRoot2', folders: [], time: 0, + testFiles: [], resource: workspaceUri, nameToRun: 'rootId2' + } + ], + testFunctions: [], + testSuites: [] + }; + const buildChildren = sinon.stub(TestDiscoveredTestParser.prototype, 'buildChildren'); + buildChildren.callsFake(() => undefined); + workspaceService + .setup(w => w.getWorkspaceFolder(typemoq.It.isAny())) + .returns(() => workspace as any) + .verifiable(typemoq.Times.once()); + parser = new TestDiscoveredTestParser(workspaceService.object); + const result = parser.parse(workspaceUri, discoveredTests as any); + assert.ok(buildChildren.calledTwice); + assert.deepEqual(expectedTests, result); + }); +});