From e1e88041d1b7b472ca0a3701501bc1e4b19bef80 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 4 Mar 2019 12:10:28 -0800 Subject: [PATCH 1/8] Display test explorer when discovery has been run --- src/test/unittests/main.unit.test.ts | 67 ++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 src/test/unittests/main.unit.test.ts diff --git a/src/test/unittests/main.unit.test.ts b/src/test/unittests/main.unit.test.ts new file mode 100644 index 000000000000..bcf6c83b2cbd --- /dev/null +++ b/src/test/unittests/main.unit.test.ts @@ -0,0 +1,67 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import * as assert from 'assert'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { Disposable, OutputChannel } from 'vscode'; +import { CommandManager } from '../../client/common/application/commandManager'; +import { DocumentManager } from '../../client/common/application/documentManager'; +import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../client/common/application/types'; +import { WorkspaceService } from '../../client/common/application/workspace'; +import { IDisposableRegistry, IOutputChannel } from '../../client/common/types'; +import { ServiceContainer } from '../../client/ioc/container'; +import { IServiceContainer } from '../../client/ioc/types'; +import { CommandSource, TEST_OUTPUT_CHANNEL } from '../../client/unittests/common/constants'; +import { Tests, TestStatus } from '../../client/unittests/common/types'; +import { TestResultDisplay } from '../../client/unittests/display/main'; +import { UnitTestManagementService } from '../../client/unittests/main'; +import { ITestResultDisplay } from '../../client/unittests/types'; +import { TestManager } from '../../client/unittests/unittest/main'; + +// tslint:disable:no-any + +suite('Unit Tests - Test Management Service', () => { + let managementService: UnitTestManagementService; + let serviceContainer: IServiceContainer; + let commandManager: ICommandManager; + setup(() => { + serviceContainer = mock(ServiceContainer); + commandManager = mock(CommandManager); + const workspaceService = mock(WorkspaceService); + const documentManager = mock(DocumentManager); + + when(serviceContainer.get(IDisposableRegistry)).thenReturn([]); + when(serviceContainer.get(IOutputChannel, TEST_OUTPUT_CHANNEL)).thenReturn({} as any); + when(serviceContainer.get(IWorkspaceService)).thenReturn(instance(workspaceService)); + when(serviceContainer.get(IDocumentManager)).thenReturn(instance(documentManager)); + when(serviceContainer.get(ICommandManager)).thenReturn(instance(commandManager)); + managementService = new UnitTestManagementService(instance(serviceContainer)); + }); + + async function contextIsSetWhenTestDiscoveryCompletes(testDiscoveryFails = false) { + const testManager = mock(TestManager); + const instanceOfTestManager = instance(testManager); + const testResultDisplay = mock(TestResultDisplay); + (instanceOfTestManager as any).then = undefined; + when(serviceContainer.get(ITestResultDisplay)).thenReturn(instance(testResultDisplay)); + when(testManager.status).thenReturn(TestStatus.Idle); + const discoveryReturnValue = testDiscoveryFails ? Promise.reject() : Promise.resolve(undefined as any); + when(testManager.discoverTests(anything(), anything(), anything(), anything())).thenReturn(discoveryReturnValue); + when(testResultDisplay.displayDiscoverStatus(anything(), anything())).thenReturn(Promise.resolve(undefined as any)); + managementService.getTestManager = () => Promise.resolve(instanceOfTestManager); + + let failed = false; + try { + await managementService.discoverTests(CommandSource.testExplorer, undefined, false, true, false); + } catch { + failed = true; + } + + assert.equal(failed, testDiscoveryFails); + verify(commandManager.executeCommand('setContext', 'testsDiscovered', true)).once(); + } + test('Context is set when tests discovery runs successfully', () => contextIsSetWhenTestDiscoveryCompletes(false)); + test('Context is set when tests discovery fails', () => contextIsSetWhenTestDiscoveryCompletes(true)); +}); From 13161e846ce8e9333f5faed35f51f707ec867d04 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 4 Mar 2019 15:31:26 -0800 Subject: [PATCH 2/8] Preserve state when adding removing tests --- .../common/managers/baseTestManager.ts | 20 ++-- .../common/services/storageService.ts | 95 ++++++++++++++++--- src/client/unittests/common/types.ts | 2 +- src/client/unittests/main.ts | 6 +- 4 files changed, 98 insertions(+), 25 deletions(-) diff --git a/src/client/unittests/common/managers/baseTestManager.ts b/src/client/unittests/common/managers/baseTestManager.ts index 97b6e2094229..b349f09f1e46 100644 --- a/src/client/unittests/common/managers/baseTestManager.ts +++ b/src/client/unittests/common/managers/baseTestManager.ts @@ -133,15 +133,15 @@ export abstract class BaseTestManager implements ITestManager { this.testResultsService.resetResults(this.tests!); } - public async discoverTests(cmdSource: CommandSource, ignoreCache: boolean = false, quietMode: boolean = false, userInitiated: boolean = false): Promise { + public async discoverTests(cmdSource: CommandSource, ignoreCache: boolean = false, quietMode: boolean = false, userInitiated: boolean = false, clearTestStatus: boolean = false): Promise { if (this.discoverTestsPromise) { return this.discoverTestsPromise; } - this.discoverTestsPromise = this._discoverTests(cmdSource, ignoreCache, quietMode, userInitiated); + this.discoverTestsPromise = this._discoverTests(cmdSource, ignoreCache, quietMode, userInitiated, clearTestStatus); this.discoverTestsPromise.catch(noop).then(() => this.discoverTestsPromise = undefined).ignoreErrors(); return this.discoverTestsPromise; } - private async _discoverTests(cmdSource: CommandSource, ignoreCache: boolean = false, quietMode: boolean = false, userInitiated: boolean = false): Promise { + private async _discoverTests(cmdSource: CommandSource, ignoreCache: boolean = false, quietMode: boolean = false, userInitiated: boolean = false, clearTestStatus: boolean = false): Promise { if (!ignoreCache && this.tests! && this.tests!.testFunctions.length > 0) { this.updateStatus(TestStatus.Idle); return Promise.resolve(this.tests!); @@ -168,8 +168,13 @@ export abstract class BaseTestManager implements ITestManager { return discoveryService .discoverTests(discoveryOptions) .then(tests => { - this.tests = tests; - this.resetTestResults(); + if (clearTestStatus) { + this.resetTestResults(); + } + const wkspace = this.workspaceService.getWorkspaceFolder(Uri.file(this.rootDirectory))!.uri; + // When storing tests, the data will be merged with existing test information. + this.testCollectionStorage.storeTests(wkspace, tests); + this.tests = tests = this.testCollectionStorage.getTests(wkspace)!; this.updateStatus(TestStatus.Idle); this.discoverTestsPromise = undefined; @@ -188,8 +193,6 @@ export abstract class BaseTestManager implements ITestManager { const testsHelper = this.serviceContainer.get(ITestsHelper); testsHelper.displayTestErrorMessage('There were some errors in discovering unit tests'); } - const wkspace = this.workspaceService.getWorkspaceFolder(Uri.file(this.rootDirectory))!.uri; - this.testCollectionStorage.storeTests(wkspace, tests); this.disposeCancellationToken(CancellationTokenType.testDiscovery); sendTelemetryEvent(EventName.UNITTEST_DISCOVER, undefined, telementryProperties); return tests; @@ -216,7 +219,7 @@ export abstract class BaseTestManager implements ITestManager { this.outputChannel.appendLine(reason.toString()); } const wkspace = this.workspaceService.getWorkspaceFolder(Uri.file(this.rootDirectory))!.uri; - this.testCollectionStorage.storeTests(wkspace, null); + this.testCollectionStorage.storeTests(wkspace, undefined); this.disposeCancellationToken(CancellationTokenType.testDiscovery); return Promise.reject(reason); }); @@ -240,7 +243,6 @@ export abstract class BaseTestManager implements ITestManager { }; if (!runFailedTests && !testsToRun) { - this.resetTestResults(); this.testsStatusUpdaterService.updateStatusAsRunning(this.workspaceFolder, this.tests); } diff --git a/src/client/unittests/common/services/storageService.ts b/src/client/unittests/common/services/storageService.ts index 4d6fbc28ddb0..ccace02d3979 100644 --- a/src/client/unittests/common/services/storageService.ts +++ b/src/client/unittests/common/services/storageService.ts @@ -1,28 +1,35 @@ import { inject, injectable } from 'inversify'; -import { Disposable, Event, EventEmitter, Uri, workspace } from 'vscode'; +import { Disposable, Event, EventEmitter, Uri } from 'vscode'; +import { IWorkspaceService } from '../../../common/application/types'; import { IDisposableRegistry } from '../../../common/types'; import { TestDataItem } from '../../types'; -import { FlattenedTestFunction, FlattenedTestSuite, ITestCollectionStorageService, TestFunction, Tests, TestSuite } from './../types'; +import { FlattenedTestFunction, FlattenedTestSuite, ITestCollectionStorageService, ITestResultsService, TestFile, TestFolder, TestFunction, Tests, TestSuite } from './../types'; @injectable() export class TestCollectionStorageService implements ITestCollectionStorageService { private _onDidChange = new EventEmitter<{ uri: Uri; data?: TestDataItem }>(); private testsIndexedByWorkspaceUri = new Map(); - constructor(@inject(IDisposableRegistry) disposables: Disposable[]) { + constructor(@inject(IDisposableRegistry) disposables: Disposable[], + @inject(ITestResultsService) private readonly resultsService: ITestResultsService, + @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService) { disposables.push(this); } public get onDidChange(): Event<{ uri: Uri; data?: TestDataItem }> { return this._onDidChange.event; } - public getTests(wkspace: Uri): Tests | undefined { - const workspaceFolder = this.getWorkspaceFolderPath(wkspace) || ''; + public getTests(resource: Uri): Tests | undefined { + const workspaceFolder = this.workspaceService.getWorkspaceFolderIdentifier(resource); return this.testsIndexedByWorkspaceUri.has(workspaceFolder) ? this.testsIndexedByWorkspaceUri.get(workspaceFolder) : undefined; } - public storeTests(wkspace: Uri, tests: Tests | undefined): void { - const workspaceFolder = this.getWorkspaceFolderPath(wkspace) || ''; + public storeTests(resource: Uri, tests: Tests | undefined): void { + const workspaceFolder = this.workspaceService.getWorkspaceFolderIdentifier(resource); + const existingTests = this.getTests(resource); + if (existingTests && tests && this.canMergeTestData(existingTests, tests)) { + this.mergeTestData(existingTests, tests); + } this.testsIndexedByWorkspaceUri.set(workspaceFolder, tests); - this._onDidChange.fire({ uri: wkspace }); + this._onDidChange.fire({ uri: resource }); } public findFlattendTestFunction(resource: Uri, func: TestFunction): FlattenedTestFunction | undefined { const tests = this.getTests(resource); @@ -41,11 +48,75 @@ export class TestCollectionStorageService implements ITestCollectionStorageServi public dispose() { this.testsIndexedByWorkspaceUri.clear(); } - public update(resource: Uri, item: TestDataItem): void{ + public update(resource: Uri, item: TestDataItem): void { this._onDidChange.fire({ uri: resource, data: item }); } - private getWorkspaceFolderPath(resource: Uri): string | undefined { - const folder = workspace.getWorkspaceFolder(resource); - return folder ? folder.uri.path : undefined; + protected canMergeTestData(existingTests: Tests, newTests: Tests): boolean { + // Adding/remoging functions or suites are allowed. + if (existingTests.testFiles.length !== newTests.testFiles.length || + existingTests.rootTestFolders.length !== newTests.rootTestFolders.length || + existingTests.testFolders.length !== newTests.testFolders.length) { + return false; + } + return true; + } + protected mergeTestData(existingTests: Tests, newTests: Tests): void { + mergeFolders(existingTests.testFolders, newTests.testFolders); + this.resultsService.updateResults(existingTests); } } + +function mergeFolders(source: TestFolder[], target: TestFolder[]): void { + source.forEach(sourceFolder => { + const targetFolder = target.find(fn => fn.name === sourceFolder.name && fn.nameToRun === sourceFolder.nameToRun); + if (!targetFolder) { + return; + } + mergeValueTypes(sourceFolder, targetFolder); + mergeFiles(sourceFolder.testFiles, targetFolder.testFiles); + }); +} +function mergeFiles(source: TestFile[], target: TestFile[]): void { + source.forEach(sourceFile => { + const targetFile = target.find(fn => fn.name === sourceFile.name && fn.nameToRun === sourceFile.nameToRun); + if (!targetFile) { + return; + } + mergeValueTypes(sourceFile, targetFile); + mergeFunctions(sourceFile.functions, targetFile.functions); + mergeSuites(sourceFile.suites, targetFile.suites); + }); +} + +function mergeFunctions(source: TestFunction[], target: TestFunction[]): void { + source.forEach(sourceFn => { + const targetFn = target.find(fn => fn.name === sourceFn.name && fn.nameToRun === sourceFn.nameToRun); + if (!targetFn) { + return; + } + mergeValueTypes(sourceFn, targetFn); + }); +} + +function mergeSuites(source: TestSuite[], target: TestSuite[]): void { + source.forEach(sourceSuite => { + const targetSuite = target.find(suite => suite.name === sourceSuite.name && + suite.nameToRun === sourceSuite.nameToRun && + suite.xmlName === sourceSuite.xmlName); + if (!targetSuite) { + return; + } + mergeValueTypes(sourceSuite, targetSuite); + mergeFunctions(sourceSuite.functions, targetSuite.functions); + mergeSuites(sourceSuite.suites, targetSuite.suites); + }); +} + +function mergeValueTypes(source: T, target: T): void { + Object.keys(source).forEach(key => { + const value = source[key]; + if (['boolean', 'number', 'string', 'undefined'].indexOf(typeof value) >= 0) { + target[key] = value; + } + }); +} diff --git a/src/client/unittests/common/types.ts b/src/client/unittests/common/types.ts index 8ca2d3a29274..979a0918246b 100644 --- a/src/client/unittests/common/types.ts +++ b/src/client/unittests/common/types.ts @@ -232,7 +232,7 @@ export interface ITestManager extends Disposable { readonly onDidStatusChange: Event; stop(): void; resetTestResults(): void; - discoverTests(cmdSource: CommandSource, ignoreCache?: boolean, quietMode?: boolean, userInitiated?: boolean): Promise; + discoverTests(cmdSource: CommandSource, ignoreCache?: boolean, quietMode?: boolean, userInitiated?: boolean, clearTestStatus?: boolean): Promise; runTest(cmdSource: CommandSource, testsToRun?: TestsToRun, runFailedTests?: boolean, debug?: boolean): Promise; } diff --git a/src/client/unittests/main.ts b/src/client/unittests/main.ts index b075ace37d69..8c348cad2b48 100644 --- a/src/client/unittests/main.ts +++ b/src/client/unittests/main.ts @@ -174,7 +174,7 @@ export class UnitTestManagementService implements IUnitTestManagementService, Di this.discoverTests(CommandSource.auto, this.workspaceService.workspaceFolders![0].uri, true).ignoreErrors(); } - public async discoverTests(cmdSource: CommandSource, resource?: Uri, ignoreCache?: boolean, userInitiated?: boolean, quietMode?: boolean) { + public async discoverTests(cmdSource: CommandSource, resource?: Uri, ignoreCache?: boolean, userInitiated?: boolean, quietMode?: boolean, clearTestStatus?: boolean) { const testManager = await this.getTestManager(true, resource); if (!testManager) { return; @@ -187,7 +187,7 @@ export class UnitTestManagementService implements IUnitTestManagementService, Di if (!this.testResultDisplay) { this.testResultDisplay = this.serviceContainer.get(ITestResultDisplay); } - const discoveryPromise = testManager.discoverTests(cmdSource, ignoreCache, quietMode, userInitiated); + const discoveryPromise = testManager.discoverTests(cmdSource, ignoreCache, quietMode, userInitiated, clearTestStatus); this.testResultDisplay.displayDiscoverStatus(discoveryPromise, quietMode) .catch(ex => console.error('Python Extension: displayDiscoverStatus', ex)); await discoveryPromise; @@ -354,7 +354,7 @@ export class UnitTestManagementService implements IUnitTestManagementService, Di commandManager.registerCommand(constants.Commands.Tests_Discover, (_, cmdSource: CommandSource = CommandSource.commandPalette, resource?: Uri) => { // Ignore the exceptions returned. // This command will be invoked from other places of the extension. - this.discoverTests(cmdSource, resource, true, true) + this.discoverTests(cmdSource, resource, true, true, false, true) .ignoreErrors(); }), commandManager.registerCommand(constants.Commands.Tests_Configure, (_, cmdSource: CommandSource = CommandSource.commandPalette, resource?: Uri) => { From 3a95db4afa951ff7f18071cec1d85b881a27d0cf Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 4 Mar 2019 16:43:53 -0800 Subject: [PATCH 3/8] Retain state of tests when auto discovering tests --- news/1 Enhancements/4576.md | 1 + .../services/storageService.unit.test.ts | 137 ++++++++++++++++++ .../unittests/common/testUtils.unit.test.ts | 24 +-- 3 files changed, 151 insertions(+), 11 deletions(-) create mode 100644 news/1 Enhancements/4576.md create mode 100644 src/test/unittests/common/services/storageService.unit.test.ts diff --git a/news/1 Enhancements/4576.md b/news/1 Enhancements/4576.md new file mode 100644 index 000000000000..c82af1e401d1 --- /dev/null +++ b/news/1 Enhancements/4576.md @@ -0,0 +1 @@ +Retain state of tests when auto discovering tests. diff --git a/src/test/unittests/common/services/storageService.unit.test.ts b/src/test/unittests/common/services/storageService.unit.test.ts new file mode 100644 index 000000000000..8193614cfc29 --- /dev/null +++ b/src/test/unittests/common/services/storageService.unit.test.ts @@ -0,0 +1,137 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import * as assert from 'assert'; +import { instance, mock } from 'ts-mockito'; +import { Uri } from 'vscode'; +import { IWorkspaceService } from '../../../../client/common/application/types'; +import { WorkspaceService } from '../../../../client/common/application/workspace'; +import { TestCollectionStorageService } from '../../../../client/unittests/common/services/storageService'; +import { TestResultsService } from '../../../../client/unittests/common/services/testResultsService'; +import { FlattenedTestFunction, FlattenedTestSuite, ITestCollectionStorageService, ITestResultsService, TestFile, TestFolder, TestFunction, Tests, TestStatus, TestSuite, TestType } from '../../../../client/unittests/common/types'; +import { createMockTestDataItem } from '../testUtils.unit.test'; + +// tslint:disable:no-any max-func-body-length +suite('Unit Tests - Storage Service', () => { + let storageService: ITestCollectionStorageService; + let resultsService: ITestResultsService; + let workspaceService: IWorkspaceService; + const workspaceUri = Uri.file(__dirname); + let testData1: Tests; + let testData2: Tests; + setup(() => { + resultsService = mock(TestResultsService); + workspaceService = mock(WorkspaceService); + storageService = new TestCollectionStorageService([], instance(resultsService), instance(workspaceService)); + setupTestData1(); + setupTestData2(); + }); + + function setupTestData1() { + const folder1 = createMockTestDataItem(TestType.testFolder, '1'); + const file1 = createMockTestDataItem(TestType.testFile, '1'); + folder1.testFiles.push(file1); + const suite1 = createMockTestDataItem(TestType.testSuite, '1'); + const suite2 = createMockTestDataItem(TestType.testSuite, '2'); + const fn1 = createMockTestDataItem(TestType.testFunction, '1'); + const fn2 = createMockTestDataItem(TestType.testFunction, '2'); + const fn3 = createMockTestDataItem(TestType.testFunction, '3'); + file1.suites.push(suite1); + file1.suites.push(suite2); + file1.functions.push(fn1); + suite1.functions.push(fn2); + suite2.functions.push(fn3); + const flattendSuite1: FlattenedTestSuite = { + testSuite: suite1, + xmlClassName: suite1.xmlName + } as any; + const flattendSuite2: FlattenedTestSuite = { + testSuite: suite2, + xmlClassName: suite2.xmlName + } as any; + const flattendFn1: FlattenedTestFunction = { + testFunction: fn1, + xmlClassName: fn1.name + } as any; + const flattendFn2: FlattenedTestFunction = { + testFunction: fn2, + xmlClassName: fn2.name + } as any; + const flattendFn3: FlattenedTestFunction = { + testFunction: fn3, + xmlClassName: fn3.name + } as any; + testData1 = { + rootTestFolders: [folder1], + summary: { errors: 0, skipped: 0, passed: 0, failures: 0 }, + testFiles: [file1], + testFolders: [folder1], + testFunctions: [flattendFn1, flattendFn2, flattendFn3], + testSuites: [flattendSuite1, flattendSuite2] + }; + } + + function setupTestData2() { + const folder1 = createMockTestDataItem(TestType.testFolder, '1'); + const file1 = createMockTestDataItem(TestType.testFile, '1'); + folder1.testFiles.push(file1); + const suite1 = createMockTestDataItem(TestType.testSuite, '1'); + const suite2 = createMockTestDataItem(TestType.testSuite, '2'); + const fn1 = createMockTestDataItem(TestType.testFunction, '1'); + const fn2 = createMockTestDataItem(TestType.testFunction, '2'); + const fn3 = createMockTestDataItem(TestType.testFunction, '3'); + file1.suites.push(suite1); + file1.suites.push(suite2); + suite1.functions.push(fn1); + suite1.functions.push(fn2); + suite2.functions.push(fn3); + const flattendSuite1: FlattenedTestSuite = { + testSuite: suite1, + xmlClassName: suite1.xmlName + } as any; + const flattendSuite2: FlattenedTestSuite = { + testSuite: suite2, + xmlClassName: suite2.xmlName + } as any; + const flattendFn1: FlattenedTestFunction = { + testFunction: fn1, + xmlClassName: fn1.name + } as any; + const flattendFn2: FlattenedTestFunction = { + testFunction: fn2, + xmlClassName: fn2.name + } as any; + const flattendFn3: FlattenedTestFunction = { + testFunction: fn3, + xmlClassName: fn3.name + } as any; + testData2 = { + rootTestFolders: [folder1], + summary: { errors: 0, skipped: 0, passed: 0, failures: 0 }, + testFiles: [file1], + testFolders: [folder1], + testFunctions: [flattendFn1, flattendFn2, flattendFn3], + testSuites: [flattendSuite1, flattendSuite2] + }; + } + + test('Merge Status from existing tests', () => { + testData1.testFunctions[0].testFunction.passed = true; + testData1.testFunctions[1].testFunction.status = TestStatus.Fail; + testData1.testFunctions[2].testFunction.time = 1234; + + assert.notDeepEqual(testData1.testFunctions[0].testFunction, testData2.testFunctions[0].testFunction); + assert.notDeepEqual(testData1.testFunctions[1].testFunction, testData2.testFunctions[1].testFunction); + assert.notDeepEqual(testData1.testFunctions[2].testFunction, testData2.testFunctions[2].testFunction); + + storageService.storeTests(workspaceUri, testData1); + storageService.storeTests(workspaceUri, testData2); + + // Function 1 is in a different suite now, hence should not get updated. + assert.notDeepEqual(testData1.testFunctions[0].testFunction, testData2.testFunctions[0].testFunction); + assert.deepEqual(testData1.testFunctions[1].testFunction, testData2.testFunctions[1].testFunction); + assert.deepEqual(testData1.testFunctions[2].testFunction, testData2.testFunctions[2].testFunction); + }); +}); diff --git a/src/test/unittests/common/testUtils.unit.test.ts b/src/test/unittests/common/testUtils.unit.test.ts index 0ebeb9aa31fb..18de9576b1db 100644 --- a/src/test/unittests/common/testUtils.unit.test.ts +++ b/src/test/unittests/common/testUtils.unit.test.ts @@ -9,36 +9,38 @@ import { getParent, getTestFile, getTestFolder, getTestFunction, getTestSuite, g import { FlattenedTestFunction, FlattenedTestSuite, TestFile, TestFolder, TestFunction, Tests, TestSuite, TestType } from '../../../client/unittests/common/types'; import { TestDataItem } from '../../../client/unittests/types'; -export function createMockTestDataItem(type: TestType) { +// tslint:disable:prefer-template + +export function createMockTestDataItem(type: TestType, nameSuffix: string = '') { const folder: TestFolder = { folders: [], - name: 'Some Folder', - nameToRun: ' Some Folder', + name: 'Some Folder' + nameSuffix, + nameToRun: ' Some Folder' + nameSuffix, testFiles: [], time: 0 }; const file: TestFile = { - name: 'Some File', - nameToRun: ' Some File', + name: 'Some File' + nameSuffix, + nameToRun: ' Some File' + nameSuffix, fullPath: __filename, - xmlName: 'some xml name', + xmlName: 'some xml name' + nameSuffix, functions: [], suites: [], time: 0 }; const func: TestFunction = { - name: 'Some Function', - nameToRun: ' Some Function', + name: 'Some Function' + nameSuffix, + nameToRun: ' Some Function' + nameSuffix, time: 0 }; const suite: TestSuite = { - name: 'Some Suite', - nameToRun: ' Some Suite', + name: 'Some Suite' + nameSuffix, + nameToRun: ' Some Suite' + nameSuffix, functions: [], isInstance: true, isUnitTest: false, suites: [], - xmlName: 'some name', + xmlName: 'some name' + nameSuffix, time: 0 }; From fc2e01c6185fbea39c5af1c9eebeddbb65444d71 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 4 Mar 2019 16:48:19 -0800 Subject: [PATCH 4/8] Delete unwanted test --- src/test/unittests/main.unit.test.ts | 67 ---------------------------- 1 file changed, 67 deletions(-) delete mode 100644 src/test/unittests/main.unit.test.ts diff --git a/src/test/unittests/main.unit.test.ts b/src/test/unittests/main.unit.test.ts deleted file mode 100644 index bcf6c83b2cbd..000000000000 --- a/src/test/unittests/main.unit.test.ts +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -'use strict'; - -import * as assert from 'assert'; -import { anything, instance, mock, verify, when } from 'ts-mockito'; -import { Disposable, OutputChannel } from 'vscode'; -import { CommandManager } from '../../client/common/application/commandManager'; -import { DocumentManager } from '../../client/common/application/documentManager'; -import { ICommandManager, IDocumentManager, IWorkspaceService } from '../../client/common/application/types'; -import { WorkspaceService } from '../../client/common/application/workspace'; -import { IDisposableRegistry, IOutputChannel } from '../../client/common/types'; -import { ServiceContainer } from '../../client/ioc/container'; -import { IServiceContainer } from '../../client/ioc/types'; -import { CommandSource, TEST_OUTPUT_CHANNEL } from '../../client/unittests/common/constants'; -import { Tests, TestStatus } from '../../client/unittests/common/types'; -import { TestResultDisplay } from '../../client/unittests/display/main'; -import { UnitTestManagementService } from '../../client/unittests/main'; -import { ITestResultDisplay } from '../../client/unittests/types'; -import { TestManager } from '../../client/unittests/unittest/main'; - -// tslint:disable:no-any - -suite('Unit Tests - Test Management Service', () => { - let managementService: UnitTestManagementService; - let serviceContainer: IServiceContainer; - let commandManager: ICommandManager; - setup(() => { - serviceContainer = mock(ServiceContainer); - commandManager = mock(CommandManager); - const workspaceService = mock(WorkspaceService); - const documentManager = mock(DocumentManager); - - when(serviceContainer.get(IDisposableRegistry)).thenReturn([]); - when(serviceContainer.get(IOutputChannel, TEST_OUTPUT_CHANNEL)).thenReturn({} as any); - when(serviceContainer.get(IWorkspaceService)).thenReturn(instance(workspaceService)); - when(serviceContainer.get(IDocumentManager)).thenReturn(instance(documentManager)); - when(serviceContainer.get(ICommandManager)).thenReturn(instance(commandManager)); - managementService = new UnitTestManagementService(instance(serviceContainer)); - }); - - async function contextIsSetWhenTestDiscoveryCompletes(testDiscoveryFails = false) { - const testManager = mock(TestManager); - const instanceOfTestManager = instance(testManager); - const testResultDisplay = mock(TestResultDisplay); - (instanceOfTestManager as any).then = undefined; - when(serviceContainer.get(ITestResultDisplay)).thenReturn(instance(testResultDisplay)); - when(testManager.status).thenReturn(TestStatus.Idle); - const discoveryReturnValue = testDiscoveryFails ? Promise.reject() : Promise.resolve(undefined as any); - when(testManager.discoverTests(anything(), anything(), anything(), anything())).thenReturn(discoveryReturnValue); - when(testResultDisplay.displayDiscoverStatus(anything(), anything())).thenReturn(Promise.resolve(undefined as any)); - managementService.getTestManager = () => Promise.resolve(instanceOfTestManager); - - let failed = false; - try { - await managementService.discoverTests(CommandSource.testExplorer, undefined, false, true, false); - } catch { - failed = true; - } - - assert.equal(failed, testDiscoveryFails); - verify(commandManager.executeCommand('setContext', 'testsDiscovered', true)).once(); - } - test('Context is set when tests discovery runs successfully', () => contextIsSetWhenTestDiscoveryCompletes(false)); - test('Context is set when tests discovery fails', () => contextIsSetWhenTestDiscoveryCompletes(true)); -}); From 34d50bde347a0e4ef461d382bf781e7efa9027ff Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 5 Mar 2019 08:55:17 -0800 Subject: [PATCH 5/8] Fix test --- src/client/unittests/common/managers/baseTestManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/unittests/common/managers/baseTestManager.ts b/src/client/unittests/common/managers/baseTestManager.ts index b349f09f1e46..2b7627e0cc23 100644 --- a/src/client/unittests/common/managers/baseTestManager.ts +++ b/src/client/unittests/common/managers/baseTestManager.ts @@ -421,7 +421,7 @@ export abstract class BaseTestManager implements ITestManager { const stackStart = message.locationStack![0]; const diagPrefix = this.unitTestDiagnosticService.getMessagePrefix(message.status!); const severity = this.unitTestDiagnosticService.getSeverity(message.severity)!; - const diagMsg = message.message!.split('\n')[0]; + const diagMsg = message.message ? message.message.split('\n')[0] : ''; const diagnostic = new Diagnostic(stackStart.location.range, `${diagPrefix ? `${diagPrefix}: ` : ''}${diagMsg}`, severity); diagnostic.code = message.code; diagnostic.source = message.provider; From 52807925b6bb83cf7c20a72cf4cbc21c5a84e244 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 5 Mar 2019 20:07:50 -0800 Subject: [PATCH 6/8] Address code review comments --- .../common/managers/baseTestManager.ts | 31 +++++--- .../common/services/storageService.ts | 79 +------------------ src/client/unittests/common/testUtils.ts | 61 +++++++++++++- .../services/storageService.unit.test.ts | 2 +- 4 files changed, 82 insertions(+), 91 deletions(-) diff --git a/src/client/unittests/common/managers/baseTestManager.ts b/src/client/unittests/common/managers/baseTestManager.ts index 2b7627e0cc23..c8750e9f77d2 100644 --- a/src/client/unittests/common/managers/baseTestManager.ts +++ b/src/client/unittests/common/managers/baseTestManager.ts @@ -14,6 +14,7 @@ import { import { ICommandManager, IWorkspaceService } from '../../../common/application/types'; import '../../../common/extensions'; import { isNotInstalledError } from '../../../common/helpers'; +import { traceError } from '../../../common/logger'; import { IFileSystem } from '../../../common/platform/types'; import { IConfigurationService, IDisposableRegistry, IInstaller, IOutputChannel, IPythonSettings, Product } from '../../../common/types'; import { getNamesAndValues } from '../../../common/utils/enum'; @@ -23,7 +24,7 @@ import { EventName } from '../../../telemetry/constants'; import { sendTelemetryEvent } from '../../../telemetry/index'; import { TestDiscoverytTelemetry, TestRunTelemetry } from '../../../telemetry/types'; import { IPythonUnitTestMessage, IUnitTestDiagnosticService, WorkspaceTestStatus } from '../../types'; -import { ITestsStatusUpdaterService } from '../types'; +import { copyTestResults } from '../testUtils'; import { CANCELLATION_REASON, CommandSource, TEST_OUTPUT_CHANNEL } from './../constants'; import { ITestCollectionStorageService, @@ -31,6 +32,7 @@ import { ITestManager, ITestResultsService, ITestsHelper, + ITestsStatusUpdaterService, TestDiscoveryOptions, TestProvider, Tests, @@ -56,19 +58,19 @@ export abstract class BaseTestManager implements ITestManager { protected get testResultsService() { return this._testResultsService; } - private testCollectionStorage: ITestCollectionStorageService; - private _testResultsService: ITestResultsService; - private commandManager: ICommandManager; - private workspaceService: IWorkspaceService; - private _outputChannel: OutputChannel; + private readonly testCollectionStorage: ITestCollectionStorageService; + private readonly _testResultsService: ITestResultsService; + private readonly commandManager: ICommandManager; + private readonly workspaceService: IWorkspaceService; + private readonly _outputChannel: OutputChannel; protected tests?: Tests; private _status: TestStatus = TestStatus.Unknown; private testDiscoveryCancellationTokenSource?: CancellationTokenSource; private testRunnerCancellationTokenSource?: CancellationTokenSource; private _installer!: IInstaller; - private testsStatusUpdaterService: ITestsStatusUpdaterService; + private readonly testsStatusUpdaterService: ITestsStatusUpdaterService; private discoverTestsPromise?: Promise; - private _onDidStatusChange = new EventEmitter(); + private readonly _onDidStatusChange = new EventEmitter(); private get installer(): IInstaller { if (!this._installer) { this._installer = this.serviceContainer.get(IInstaller); @@ -77,7 +79,7 @@ export abstract class BaseTestManager implements ITestManager { } constructor( public readonly testProvider: TestProvider, - private product: Product, + private readonly product: Product, public readonly workspaceFolder: Uri, protected rootDirectory: string, protected serviceContainer: IServiceContainer @@ -168,13 +170,16 @@ export abstract class BaseTestManager implements ITestManager { return discoveryService .discoverTests(discoveryOptions) .then(tests => { + const wkspace = this.workspaceService.getWorkspaceFolder(Uri.file(this.rootDirectory))!.uri; + const existingTests = this.testCollectionStorage.getTests(wkspace)!; if (clearTestStatus) { this.resetTestResults(); + } else if (existingTests) { + copyTestResults(existingTests, tests); + this._testResultsService.updateResults(tests); } - const wkspace = this.workspaceService.getWorkspaceFolder(Uri.file(this.rootDirectory))!.uri; - // When storing tests, the data will be merged with existing test information. this.testCollectionStorage.storeTests(wkspace, tests); - this.tests = tests = this.testCollectionStorage.getTests(wkspace)!; + this.tests = tests; this.updateStatus(TestStatus.Idle); this.discoverTestsPromise = undefined; @@ -203,7 +208,7 @@ export abstract class BaseTestManager implements ITestManager { } if (isNotInstalledError(reason as Error) && !quietMode) { this.installer.promptToInstall(this.product, this.workspaceFolder) - .catch(ex => console.error('Python Extension: isNotInstalledError', ex)); + .catch(ex => traceError('isNotInstalledError', ex)); } this.tests = undefined; diff --git a/src/client/unittests/common/services/storageService.ts b/src/client/unittests/common/services/storageService.ts index ccace02d3979..bcb8de052a69 100644 --- a/src/client/unittests/common/services/storageService.ts +++ b/src/client/unittests/common/services/storageService.ts @@ -3,15 +3,14 @@ import { Disposable, Event, EventEmitter, Uri } from 'vscode'; import { IWorkspaceService } from '../../../common/application/types'; import { IDisposableRegistry } from '../../../common/types'; import { TestDataItem } from '../../types'; -import { FlattenedTestFunction, FlattenedTestSuite, ITestCollectionStorageService, ITestResultsService, TestFile, TestFolder, TestFunction, Tests, TestSuite } from './../types'; +import { FlattenedTestFunction, FlattenedTestSuite, ITestCollectionStorageService, TestFunction, Tests, TestSuite } from './../types'; @injectable() export class TestCollectionStorageService implements ITestCollectionStorageService { - private _onDidChange = new EventEmitter<{ uri: Uri; data?: TestDataItem }>(); - private testsIndexedByWorkspaceUri = new Map(); + private readonly _onDidChange = new EventEmitter<{ uri: Uri; data?: TestDataItem }>(); + private readonly testsIndexedByWorkspaceUri = new Map(); constructor(@inject(IDisposableRegistry) disposables: Disposable[], - @inject(ITestResultsService) private readonly resultsService: ITestResultsService, @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService) { disposables.push(this); } @@ -24,10 +23,6 @@ export class TestCollectionStorageService implements ITestCollectionStorageServi } public storeTests(resource: Uri, tests: Tests | undefined): void { const workspaceFolder = this.workspaceService.getWorkspaceFolderIdentifier(resource); - const existingTests = this.getTests(resource); - if (existingTests && tests && this.canMergeTestData(existingTests, tests)) { - this.mergeTestData(existingTests, tests); - } this.testsIndexedByWorkspaceUri.set(workspaceFolder, tests); this._onDidChange.fire({ uri: resource }); } @@ -51,72 +46,4 @@ export class TestCollectionStorageService implements ITestCollectionStorageServi public update(resource: Uri, item: TestDataItem): void { this._onDidChange.fire({ uri: resource, data: item }); } - protected canMergeTestData(existingTests: Tests, newTests: Tests): boolean { - // Adding/remoging functions or suites are allowed. - if (existingTests.testFiles.length !== newTests.testFiles.length || - existingTests.rootTestFolders.length !== newTests.rootTestFolders.length || - existingTests.testFolders.length !== newTests.testFolders.length) { - return false; - } - return true; - } - protected mergeTestData(existingTests: Tests, newTests: Tests): void { - mergeFolders(existingTests.testFolders, newTests.testFolders); - this.resultsService.updateResults(existingTests); - } -} - -function mergeFolders(source: TestFolder[], target: TestFolder[]): void { - source.forEach(sourceFolder => { - const targetFolder = target.find(fn => fn.name === sourceFolder.name && fn.nameToRun === sourceFolder.nameToRun); - if (!targetFolder) { - return; - } - mergeValueTypes(sourceFolder, targetFolder); - mergeFiles(sourceFolder.testFiles, targetFolder.testFiles); - }); -} -function mergeFiles(source: TestFile[], target: TestFile[]): void { - source.forEach(sourceFile => { - const targetFile = target.find(fn => fn.name === sourceFile.name && fn.nameToRun === sourceFile.nameToRun); - if (!targetFile) { - return; - } - mergeValueTypes(sourceFile, targetFile); - mergeFunctions(sourceFile.functions, targetFile.functions); - mergeSuites(sourceFile.suites, targetFile.suites); - }); -} - -function mergeFunctions(source: TestFunction[], target: TestFunction[]): void { - source.forEach(sourceFn => { - const targetFn = target.find(fn => fn.name === sourceFn.name && fn.nameToRun === sourceFn.nameToRun); - if (!targetFn) { - return; - } - mergeValueTypes(sourceFn, targetFn); - }); -} - -function mergeSuites(source: TestSuite[], target: TestSuite[]): void { - source.forEach(sourceSuite => { - const targetSuite = target.find(suite => suite.name === sourceSuite.name && - suite.nameToRun === sourceSuite.nameToRun && - suite.xmlName === sourceSuite.xmlName); - if (!targetSuite) { - return; - } - mergeValueTypes(sourceSuite, targetSuite); - mergeFunctions(sourceSuite.functions, targetSuite.functions); - mergeSuites(sourceSuite.suites, targetSuite.suites); - }); -} - -function mergeValueTypes(source: T, target: T): void { - Object.keys(source).forEach(key => { - const value = source[key]; - if (['boolean', 'number', 'string', 'undefined'].indexOf(typeof value) >= 0) { - target[key] = value; - } - }); } diff --git a/src/client/unittests/common/testUtils.ts b/src/client/unittests/common/testUtils.ts index ffe92e2657d3..6312119a71a3 100644 --- a/src/client/unittests/common/testUtils.ts +++ b/src/client/unittests/common/testUtils.ts @@ -54,7 +54,7 @@ export class TestsHelper implements ITestsHelper { private readonly appShell: IApplicationShell; private readonly commandManager: ICommandManager; constructor( - @inject(ITestVisitor) @named('TestFlatteningVisitor') private flatteningVisitor: TestFlatteningVisitor, + @inject(ITestVisitor) @named('TestFlatteningVisitor') private readonly flatteningVisitor: TestFlatteningVisitor, @inject(IServiceContainer) serviceContainer: IServiceContainer ) { this.appShell = serviceContainer.get(IApplicationShell); @@ -451,3 +451,62 @@ export function getChildren(item: TestDataItem): TestDataItem[] { } } } + +export function copyTestResults(source: Tests, target: Tests): void { + copyResultsForFolders(source.testFolders, target.testFolders); +} + +function copyResultsForFolders(source: TestFolder[], target: TestFolder[]): void { + source.forEach(sourceFolder => { + const targetFolder = target.find(folder => folder.name === sourceFolder.name && folder.nameToRun === sourceFolder.nameToRun); + if (!targetFolder) { + return; + } + copyValueTypes(sourceFolder, targetFolder); + copyResultsForFiles(sourceFolder.testFiles, targetFolder.testFiles); + }); +} +function copyResultsForFiles(source: TestFile[], target: TestFile[]): void { + source.forEach(sourceFile => { + const targetFile = target.find(file => file.name === sourceFile.name && file.nameToRun === sourceFile.nameToRun); + if (!targetFile) { + return; + } + copyValueTypes(sourceFile, targetFile); + copyResultsForFunctions(sourceFile.functions, targetFile.functions); + copyResultsForSuites(sourceFile.suites, targetFile.suites); + }); +} + +function copyResultsForFunctions(source: TestFunction[], target: TestFunction[]): void { + source.forEach(sourceFn => { + const targetFn = target.find(fn => fn.name === sourceFn.name && fn.nameToRun === sourceFn.nameToRun); + if (!targetFn) { + return; + } + copyValueTypes(sourceFn, targetFn); + }); +} + +function copyResultsForSuites(source: TestSuite[], target: TestSuite[]): void { + source.forEach(sourceSuite => { + const targetSuite = target.find(suite => suite.name === sourceSuite.name && + suite.nameToRun === sourceSuite.nameToRun && + suite.xmlName === sourceSuite.xmlName); + if (!targetSuite) { + return; + } + copyValueTypes(sourceSuite, targetSuite); + copyResultsForFunctions(sourceSuite.functions, targetSuite.functions); + copyResultsForSuites(sourceSuite.suites, targetSuite.suites); + }); +} + +function copyValueTypes(source: T, target: T): void { + Object.keys(source).forEach(key => { + const value = source[key]; + if (['boolean', 'number', 'string', 'undefined'].indexOf(typeof value) >= 0) { + target[key] = value; + } + }); +} diff --git a/src/test/unittests/common/services/storageService.unit.test.ts b/src/test/unittests/common/services/storageService.unit.test.ts index 8193614cfc29..b5158e5dbe7d 100644 --- a/src/test/unittests/common/services/storageService.unit.test.ts +++ b/src/test/unittests/common/services/storageService.unit.test.ts @@ -24,7 +24,7 @@ suite('Unit Tests - Storage Service', () => { setup(() => { resultsService = mock(TestResultsService); workspaceService = mock(WorkspaceService); - storageService = new TestCollectionStorageService([], instance(resultsService), instance(workspaceService)); + storageService = new TestCollectionStorageService([], instance(workspaceService)); setupTestData1(); setupTestData2(); }); From c161a271e0aca73832ca31d7c92d937b5d28a29d Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 5 Mar 2019 20:09:52 -0800 Subject: [PATCH 7/8] Remove unnecessary file --- pythonFilestesting_tools/adapters/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 pythonFilestesting_tools/adapters/__init__.py diff --git a/pythonFilestesting_tools/adapters/__init__.py b/pythonFilestesting_tools/adapters/__init__.py deleted file mode 100644 index e69de29bb2d1..000000000000 From 189bd5b58602bb9924ea7737e0b46c8d8a78f453 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 7 Mar 2019 09:10:05 -0800 Subject: [PATCH 8/8] Fixed broken tests --- .../services/storageService.unit.test.ts | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/src/test/unittests/common/services/storageService.unit.test.ts b/src/test/unittests/common/services/storageService.unit.test.ts index b5158e5dbe7d..2817fa03902f 100644 --- a/src/test/unittests/common/services/storageService.unit.test.ts +++ b/src/test/unittests/common/services/storageService.unit.test.ts @@ -4,27 +4,15 @@ 'use strict'; import * as assert from 'assert'; -import { instance, mock } from 'ts-mockito'; -import { Uri } from 'vscode'; -import { IWorkspaceService } from '../../../../client/common/application/types'; -import { WorkspaceService } from '../../../../client/common/application/workspace'; -import { TestCollectionStorageService } from '../../../../client/unittests/common/services/storageService'; -import { TestResultsService } from '../../../../client/unittests/common/services/testResultsService'; -import { FlattenedTestFunction, FlattenedTestSuite, ITestCollectionStorageService, ITestResultsService, TestFile, TestFolder, TestFunction, Tests, TestStatus, TestSuite, TestType } from '../../../../client/unittests/common/types'; +import { copyTestResults } from '../../../../client/unittests/common/testUtils'; +import { FlattenedTestFunction, FlattenedTestSuite, TestFile, TestFolder, TestFunction, Tests, TestStatus, TestSuite, TestType } from '../../../../client/unittests/common/types'; import { createMockTestDataItem } from '../testUtils.unit.test'; // tslint:disable:no-any max-func-body-length suite('Unit Tests - Storage Service', () => { - let storageService: ITestCollectionStorageService; - let resultsService: ITestResultsService; - let workspaceService: IWorkspaceService; - const workspaceUri = Uri.file(__dirname); let testData1: Tests; let testData2: Tests; setup(() => { - resultsService = mock(TestResultsService); - workspaceService = mock(WorkspaceService); - storageService = new TestCollectionStorageService([], instance(workspaceService)); setupTestData1(); setupTestData2(); }); @@ -126,8 +114,7 @@ suite('Unit Tests - Storage Service', () => { assert.notDeepEqual(testData1.testFunctions[1].testFunction, testData2.testFunctions[1].testFunction); assert.notDeepEqual(testData1.testFunctions[2].testFunction, testData2.testFunctions[2].testFunction); - storageService.storeTests(workspaceUri, testData1); - storageService.storeTests(workspaceUri, testData2); + copyTestResults(testData1, testData2); // Function 1 is in a different suite now, hence should not get updated. assert.notDeepEqual(testData1.testFunctions[0].testFunction, testData2.testFunctions[0].testFunction);