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/pythonFilestesting_tools/adapters/__init__.py b/pythonFilestesting_tools/adapters/__init__.py deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/src/client/unittests/common/managers/baseTestManager.ts b/src/client/unittests/common/managers/baseTestManager.ts index 97b6e2094229..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 @@ -133,15 +135,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 +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); + } + this.testCollectionStorage.storeTests(wkspace, tests); this.tests = tests; - this.resetTestResults(); this.updateStatus(TestStatus.Idle); this.discoverTestsPromise = undefined; @@ -188,8 +198,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; @@ -200,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; @@ -216,7 +224,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 +248,6 @@ export abstract class BaseTestManager implements ITestManager { }; if (!runFailedTests && !testsToRun) { - this.resetTestResults(); this.testsStatusUpdaterService.updateStatusAsRunning(this.workspaceFolder, this.tests); } @@ -419,7 +426,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; diff --git a/src/client/unittests/common/services/storageService.ts b/src/client/unittests/common/services/storageService.ts index 4d6fbc28ddb0..bcb8de052a69 100644 --- a/src/client/unittests/common/services/storageService.ts +++ b/src/client/unittests/common/services/storageService.ts @@ -1,28 +1,30 @@ 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'; @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[]) { + constructor(@inject(IDisposableRegistry) disposables: Disposable[], + @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); 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 +43,7 @@ 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; - } } 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/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) => { 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..2817fa03902f --- /dev/null +++ b/src/test/unittests/common/services/storageService.unit.test.ts @@ -0,0 +1,124 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import * as assert from 'assert'; +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 testData1: Tests; + let testData2: Tests; + setup(() => { + 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); + + 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); + 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 };