From 21f560c128167578d55f9b2b94001e5b6e80c186 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 6 Feb 2019 01:09:56 -0700 Subject: [PATCH 1/8] Add workspace test discovery data to the Python Test tree view. For #4273 - Make Python Test View Provider injectable (testable) - Maps `Tests` object to TreeViewItem hierarchy - Adds a Event (subscription) model to the TestStorageService --- src/client/extension.ts | 8 +- src/client/providers/serviceRegistry.ts | 6 +- src/client/providers/testTreeViewItem.ts | 100 +++++++++++++++++- src/client/providers/testTreeViewProvider.ts | 91 ++++++++-------- src/client/providers/types.ts | 13 ++- .../common/services/storageService.ts | 12 ++- src/client/unittests/common/types.ts | 6 +- 7 files changed, 184 insertions(+), 52 deletions(-) diff --git a/src/client/extension.ts b/src/client/extension.ts index cfafac2957e7..41811311c882 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -93,8 +93,9 @@ import { ReplProvider } from './providers/replProvider'; import { registerTypes as providersRegisterTypes } from './providers/serviceRegistry'; import { activateSimplePythonRefactorProvider } from './providers/simpleRefactorProvider'; import { TerminalProvider } from './providers/terminalProvider'; -import { PythonTestTreeViewProvider } from './providers/testTreeViewProvider'; -import { ISortImportsEditingProvider } from './providers/types'; +import { + IPythonTestTreeViewProvider, ISortImportsEditingProvider +} from './providers/types'; import { activateUpdateSparkLibraryProvider } from './providers/updateSparkLibraryProvider'; import { sendTelemetryEvent } from './telemetry'; import { EventName } from './telemetry/constants'; @@ -208,7 +209,8 @@ async function activateUnsafe(context: ExtensionContext): Promise context.subscriptions.push(languages.registerCodeActionsProvider(PYTHON, new PythonCodeActionProvider(), { providedCodeActionKinds: [CodeActionKind.SourceOrganizeImports] })); - context.subscriptions.push(window.registerTreeDataProvider('python_tests', new PythonTestTreeViewProvider())); + const testViewProvider = serviceContainer.get(IPythonTestTreeViewProvider); + context.subscriptions.push(window.registerTreeDataProvider('python_tests', testViewProvider)); serviceContainer.getAll(IDebugConfigurationService).forEach(debugConfigProvider => { context.subscriptions.push(debug.registerDebugConfigurationProvider(DebuggerTypeName, debugConfigProvider)); diff --git a/src/client/providers/serviceRegistry.ts b/src/client/providers/serviceRegistry.ts index 7418e0175e51..244b40c9002a 100644 --- a/src/client/providers/serviceRegistry.ts +++ b/src/client/providers/serviceRegistry.ts @@ -5,8 +5,12 @@ import { IServiceManager } from '../ioc/types'; import { SortImportsEditingProvider } from './importSortProvider'; -import { ISortImportsEditingProvider } from './types'; +import { PythonTestTreeViewProvider } from './testTreeViewProvider'; +import { + IPythonTestTreeViewProvider, ISortImportsEditingProvider +} from './types'; export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(ISortImportsEditingProvider, SortImportsEditingProvider); + serviceManager.addSingleton(IPythonTestTreeViewProvider, PythonTestTreeViewProvider); } diff --git a/src/client/providers/testTreeViewItem.ts b/src/client/providers/testTreeViewItem.ts index 0c95c8ea52a9..354fff460785 100644 --- a/src/client/providers/testTreeViewItem.ts +++ b/src/client/providers/testTreeViewItem.ts @@ -6,7 +6,10 @@ import { TreeItem, TreeItemCollapsibleState } from 'vscode'; -import { TestStatus } from '../unittests/common/types'; +import { + TestFile, TestFolder, TestFunction, + TestStatus, TestSuite +} from '../unittests/common/types'; export enum PythonTestTreeItemType { Root = 'Root', @@ -24,7 +27,9 @@ export class PythonTestTreeItem extends TreeItem { private myChildren: PythonTestTreeItem[], runId: string, name: string, - testStatus: TestStatus = TestStatus.Unknown) { + testStatus: TestStatus = TestStatus.Unknown, + private data: TestFolder | TestFile | TestSuite | TestFunction + ) { super( `[${kind}] ${name}`, @@ -36,6 +41,97 @@ export class PythonTestTreeItem extends TreeItem { this.tooltip = `Status: ${testStatus}`; } + public static createFromFolder( + folder: TestFolder, + parent?: PythonTestTreeItem + ): PythonTestTreeItem { + + const folderItem = new PythonTestTreeItem( + PythonTestTreeItemType.Package, + parent, + [], + folder.nameToRun, + folder.name, + folder.status, + folder + ); + + folder.testFiles.forEach((testFile: TestFile) => { + folderItem.children.push(PythonTestTreeItem.createFromFile(testFile, folderItem)); + }); + + return folderItem; + } + + public static createFromFile( + testFile: TestFile, + parent?: PythonTestTreeItem + ): PythonTestTreeItem { + + const fileItem = new PythonTestTreeItem( + PythonTestTreeItemType.File, + parent, + [], + testFile.nameToRun, + testFile.name, + testFile.status, + testFile + ); + + testFile.functions.forEach((fn: TestFunction) => { + fileItem.children.push(PythonTestTreeItem.createFromFunction(fn, fileItem)); + }); + testFile.suites.forEach((suite: TestSuite) => { + fileItem.children.push(PythonTestTreeItem.createFromSuite(suite, fileItem)); + }); + + return fileItem; + } + + public static createFromSuite( + suite: TestSuite, + parent: PythonTestTreeItem + ): PythonTestTreeItem { + + const suiteItem = new PythonTestTreeItem( + PythonTestTreeItemType.Suite, + parent, + [], + suite.nameToRun, + suite.name, + suite.status, + suite + ); + + suite.suites.forEach((subSuite: TestSuite) => { + suiteItem.children.push(PythonTestTreeItem.createFromSuite(subSuite, suiteItem)); + }); + suite.functions.forEach((fn: TestFunction) => { + suiteItem.children.push(PythonTestTreeItem.createFromFunction(fn, suiteItem)); + }); + + return suiteItem; + } + + public static createFromFunction( + fn: TestFunction, + parent: PythonTestTreeItem + ): PythonTestTreeItem { + + // tslint:disable-next-line:no-unnecessary-local-variable + const funcItem = new PythonTestTreeItem( + PythonTestTreeItemType.Function, + parent, + undefined, + fn.nameToRun, + fn.name, + fn.status, + fn + ); + + return funcItem; + } + public get children(): PythonTestTreeItem[] { return this.myChildren; } diff --git a/src/client/providers/testTreeViewProvider.ts b/src/client/providers/testTreeViewProvider.ts index 1dda3d5733aa..7d2d59d436c0 100644 --- a/src/client/providers/testTreeViewProvider.ts +++ b/src/client/providers/testTreeViewProvider.ts @@ -3,16 +3,23 @@ 'use strict'; +import { inject, injectable } from 'inversify'; import { - Event, EventEmitter, - ProviderResult, TreeDataProvider + Event, EventEmitter, ProviderResult, Uri } from 'vscode'; +import { IWorkspaceService } from '../common/application/types'; +import { traceDecorators } from '../common/logger'; +import { IDisposable, IDisposableRegistry, Resource } from '../common/types'; import { - PythonTestTreeItem, - PythonTestTreeItemType + ITestCollectionStorageService, TestFolder, Tests, TestStatus +} from '../unittests/common/types'; +import { + PythonTestTreeItem, PythonTestTreeItemType } from './testTreeViewItem'; +import { IPythonTestTreeViewProvider } from './types'; -export class PythonTestTreeViewProvider implements TreeDataProvider { +@injectable() +export class PythonTestTreeViewProvider implements IPythonTestTreeViewProvider, IDisposable { /** * This will trigger the view to update the changed element/root and its children recursively (if shown). * To signal that root has changed, do not pass any argument or pass `undefined` or `null`. @@ -21,11 +28,24 @@ export class PythonTestTreeViewProvider implements TreeDataProvider = new EventEmitter(); private root: PythonTestTreeItem[]; + private disposables: IDisposable[] = []; - constructor() { + constructor( + @inject(ITestCollectionStorageService) private testStore: ITestCollectionStorageService, + @inject(IWorkspaceService) private workspace: IWorkspaceService, + @inject(IDisposableRegistry) disposableRegistry: IDisposableRegistry + ) { this.onDidChangeTreeData = this._onDidChangeTreeData.event; - // set up some dummy data to just show that the test explorer loads. - this.root = this.getTestTree(); + this.root = [new PythonTestTreeItem(PythonTestTreeItemType.Root, undefined, undefined, '*', 'no tests discovered yet', TestStatus.Unknown, undefined)]; + this.refresh(this.workspace.workspaceFolders[0].uri); + disposableRegistry.push(this); + this.disposables.push(this.testStore.onTestStoreUpdated(this.onTestStoreUpdated, this)); + } + + // tslint:disable-next-line:no-empty + public dispose() { + this.disposables.forEach(d => d.dispose()); + this._onDidChangeTreeData.dispose(); } /** @@ -64,42 +84,27 @@ export class PythonTestTreeViewProvider implements TreeDataProvider { + newRoot.push(PythonTestTreeItem.createFromFolder(tf)); + }); + this.root = newRoot; + this._onDidChangeTreeData.fire(); + } + } - return roots; + @traceDecorators.verbose('>>> DEREK >>> Test store is being updated...') + private onTestStoreUpdated(workspace: Uri): void { + this.refresh(workspace); } } diff --git a/src/client/providers/types.ts b/src/client/providers/types.ts index f2d1bc6eea3a..1693e71fa50c 100644 --- a/src/client/providers/types.ts +++ b/src/client/providers/types.ts @@ -3,7 +3,11 @@ 'use strict'; -import { CancellationToken, Uri, WorkspaceEdit } from 'vscode'; +import { + CancellationToken, Event, ProviderResult, + TreeDataProvider, Uri, WorkspaceEdit +} from 'vscode'; +import { PythonTestTreeItem } from './testTreeViewItem'; export const ISortImportsEditingProvider = Symbol('ISortImportsEditingProvider'); export interface ISortImportsEditingProvider { @@ -11,3 +15,10 @@ export interface ISortImportsEditingProvider { sortImports(uri?: Uri): Promise; registerCommands(): void; } + +export const IPythonTestTreeViewProvider = Symbol('IPythonTestTreeViewProvider'); +export interface IPythonTestTreeViewProvider extends TreeDataProvider { + onDidChangeTreeData: Event; + getTreeItem(element: PythonTestTreeItem): Promise; + getChildren(element?: PythonTestTreeItem): ProviderResult; +} diff --git a/src/client/unittests/common/services/storageService.ts b/src/client/unittests/common/services/storageService.ts index 5f4843ebf931..685c729095a3 100644 --- a/src/client/unittests/common/services/storageService.ts +++ b/src/client/unittests/common/services/storageService.ts @@ -1,13 +1,21 @@ import { inject, injectable } from 'inversify'; -import { Disposable, Uri, workspace } from 'vscode'; +import { + Disposable, Event, EventEmitter, + Uri, workspace +} from 'vscode'; import { IDisposableRegistry } from '../../../common/types'; import { FlattenedTestFunction, FlattenedTestSuite, ITestCollectionStorageService, TestFunction, Tests, TestSuite } from './../types'; @injectable() export class TestCollectionStorageService implements ITestCollectionStorageService { + public readonly onTestStoreUpdated: Event; + private testsIndexedByWorkspaceUri = new Map(); + private _onTestStoreUpdated: EventEmitter = new EventEmitter(); + constructor(@inject(IDisposableRegistry) disposables: Disposable[]) { disposables.push(this); + this.onTestStoreUpdated = this._onTestStoreUpdated.event; } public getTests(wkspace: Uri): Tests | undefined { const workspaceFolder = this.getWorkspaceFolderPath(wkspace) || ''; @@ -16,6 +24,7 @@ export class TestCollectionStorageService implements ITestCollectionStorageServi public storeTests(wkspace: Uri, tests: Tests | undefined): void { const workspaceFolder = this.getWorkspaceFolderPath(wkspace) || ''; this.testsIndexedByWorkspaceUri.set(workspaceFolder, tests); + this._onTestStoreUpdated.fire(wkspace); } public findFlattendTestFunction(resource: Uri, func: TestFunction): FlattenedTestFunction | undefined { const tests = this.getTests(resource); @@ -33,6 +42,7 @@ export class TestCollectionStorageService implements ITestCollectionStorageServi } public dispose() { this.testsIndexedByWorkspaceUri.clear(); + this._onTestStoreUpdated.dispose(); } private getWorkspaceFolderPath(resource: Uri): string | undefined { const folder = workspace.getWorkspaceFolder(resource); diff --git a/src/client/unittests/common/types.ts b/src/client/unittests/common/types.ts index c7c758b20623..8200d0a00b2c 100644 --- a/src/client/unittests/common/types.ts +++ b/src/client/unittests/common/types.ts @@ -1,4 +1,7 @@ -import { CancellationToken, DiagnosticCollection, Disposable, OutputChannel, Uri } from 'vscode'; +import { + CancellationToken, DiagnosticCollection, Disposable, + Event, OutputChannel, Uri +} from 'vscode'; import { IUnitTestSettings, Product } from '../../common/types'; import { IPythonUnitTestMessage } from '../types'; import { CommandSource } from './constants'; @@ -181,6 +184,7 @@ export interface ITestVisitor { export const ITestCollectionStorageService = Symbol('ITestCollectionStorageService'); export interface ITestCollectionStorageService extends Disposable { + onTestStoreUpdated: Event; getTests(wkspace: Uri): Tests | undefined; storeTests(wkspace: Uri, tests: Tests | null | undefined): void; findFlattendTestFunction(resource: Uri, func: TestFunction): FlattenedTestFunction | undefined; From 9797fd3494ed69782ae3ed434f0d4127ffa53514 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 6 Feb 2019 11:56:05 -0800 Subject: [PATCH 2/8] Hygiene fix --- src/client/providers/testTreeViewItem.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/providers/testTreeViewItem.ts b/src/client/providers/testTreeViewItem.ts index 354fff460785..916908f7be66 100644 --- a/src/client/providers/testTreeViewItem.ts +++ b/src/client/providers/testTreeViewItem.ts @@ -28,6 +28,7 @@ export class PythonTestTreeItem extends TreeItem { runId: string, name: string, testStatus: TestStatus = TestStatus.Unknown, + // tslint:disable-next-line: private data: TestFolder | TestFile | TestSuite | TestFunction ) { From a6bd7080eb722780c67105bf7f8f5902bd7db40f Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 6 Feb 2019 12:04:23 -0800 Subject: [PATCH 3/8] argh hygiene --- src/client/providers/testTreeViewItem.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/providers/testTreeViewItem.ts b/src/client/providers/testTreeViewItem.ts index 916908f7be66..29ffa664a02e 100644 --- a/src/client/providers/testTreeViewItem.ts +++ b/src/client/providers/testTreeViewItem.ts @@ -28,7 +28,7 @@ export class PythonTestTreeItem extends TreeItem { runId: string, name: string, testStatus: TestStatus = TestStatus.Unknown, - // tslint:disable-next-line: + // tslint:disable-next-line:no-unused-variable private data: TestFolder | TestFile | TestSuite | TestFunction ) { From fa11744e13c3c89edf714a1e39f7c60c60ee6a69 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 6 Feb 2019 14:03:45 -0800 Subject: [PATCH 4/8] PR review suggestions - Ensure all properties in the TestTreeItem are RO - Remove `Python` from the names of classes/enums --- src/client/providers/serviceRegistry.ts | 6 +- src/client/providers/testTreeViewItem.ts | 60 ++++++++++---------- src/client/providers/testTreeViewProvider.ts | 24 ++++---- src/client/providers/types.ts | 12 ++-- 4 files changed, 51 insertions(+), 51 deletions(-) diff --git a/src/client/providers/serviceRegistry.ts b/src/client/providers/serviceRegistry.ts index 244b40c9002a..2c5b49ee4304 100644 --- a/src/client/providers/serviceRegistry.ts +++ b/src/client/providers/serviceRegistry.ts @@ -5,12 +5,12 @@ import { IServiceManager } from '../ioc/types'; import { SortImportsEditingProvider } from './importSortProvider'; -import { PythonTestTreeViewProvider } from './testTreeViewProvider'; +import { TestTreeViewProvider } from './testTreeViewProvider'; import { - IPythonTestTreeViewProvider, ISortImportsEditingProvider + ISortImportsEditingProvider, ITestTreeViewProvider } from './types'; export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(ISortImportsEditingProvider, SortImportsEditingProvider); - serviceManager.addSingleton(IPythonTestTreeViewProvider, PythonTestTreeViewProvider); + serviceManager.addSingleton(ITestTreeViewProvider, TestTreeViewProvider); } diff --git a/src/client/providers/testTreeViewItem.ts b/src/client/providers/testTreeViewItem.ts index 29ffa664a02e..1107244f831c 100644 --- a/src/client/providers/testTreeViewItem.ts +++ b/src/client/providers/testTreeViewItem.ts @@ -11,7 +11,7 @@ import { TestStatus, TestSuite } from '../unittests/common/types'; -export enum PythonTestTreeItemType { +export enum TestTreeItemType { Root = 'Root', Package = 'Package', File = 'File', @@ -19,22 +19,22 @@ export enum PythonTestTreeItemType { Function = 'Function' } -export class PythonTestTreeItem extends TreeItem { +export class TestTreeItem extends TreeItem { constructor( - kind: PythonTestTreeItemType, - private myParent: PythonTestTreeItem, - private myChildren: PythonTestTreeItem[], + kind: TestTreeItemType, + private readonly myParent: TestTreeItem, + private readonly myChildren: TestTreeItem[], runId: string, name: string, testStatus: TestStatus = TestStatus.Unknown, // tslint:disable-next-line:no-unused-variable - private data: TestFolder | TestFile | TestSuite | TestFunction + private readonly data: TestFolder | TestFile | TestSuite | TestFunction ) { super( `[${kind}] ${name}`, - kind === PythonTestTreeItemType.Function ? TreeItemCollapsibleState.None : TreeItemCollapsibleState.Collapsed + kind === TestTreeItemType.Function ? TreeItemCollapsibleState.None : TreeItemCollapsibleState.Collapsed ); this.contextValue = kind; @@ -44,11 +44,11 @@ export class PythonTestTreeItem extends TreeItem { public static createFromFolder( folder: TestFolder, - parent?: PythonTestTreeItem - ): PythonTestTreeItem { + parent?: TestTreeItem + ): TestTreeItem { - const folderItem = new PythonTestTreeItem( - PythonTestTreeItemType.Package, + const folderItem = new TestTreeItem( + TestTreeItemType.Package, parent, [], folder.nameToRun, @@ -58,7 +58,7 @@ export class PythonTestTreeItem extends TreeItem { ); folder.testFiles.forEach((testFile: TestFile) => { - folderItem.children.push(PythonTestTreeItem.createFromFile(testFile, folderItem)); + folderItem.children.push(TestTreeItem.createFromFile(testFile, folderItem)); }); return folderItem; @@ -66,11 +66,11 @@ export class PythonTestTreeItem extends TreeItem { public static createFromFile( testFile: TestFile, - parent?: PythonTestTreeItem - ): PythonTestTreeItem { + parent?: TestTreeItem + ): TestTreeItem { - const fileItem = new PythonTestTreeItem( - PythonTestTreeItemType.File, + const fileItem = new TestTreeItem( + TestTreeItemType.File, parent, [], testFile.nameToRun, @@ -80,10 +80,10 @@ export class PythonTestTreeItem extends TreeItem { ); testFile.functions.forEach((fn: TestFunction) => { - fileItem.children.push(PythonTestTreeItem.createFromFunction(fn, fileItem)); + fileItem.children.push(TestTreeItem.createFromFunction(fn, fileItem)); }); testFile.suites.forEach((suite: TestSuite) => { - fileItem.children.push(PythonTestTreeItem.createFromSuite(suite, fileItem)); + fileItem.children.push(TestTreeItem.createFromSuite(suite, fileItem)); }); return fileItem; @@ -91,11 +91,11 @@ export class PythonTestTreeItem extends TreeItem { public static createFromSuite( suite: TestSuite, - parent: PythonTestTreeItem - ): PythonTestTreeItem { + parent: TestTreeItem + ): TestTreeItem { - const suiteItem = new PythonTestTreeItem( - PythonTestTreeItemType.Suite, + const suiteItem = new TestTreeItem( + TestTreeItemType.Suite, parent, [], suite.nameToRun, @@ -105,10 +105,10 @@ export class PythonTestTreeItem extends TreeItem { ); suite.suites.forEach((subSuite: TestSuite) => { - suiteItem.children.push(PythonTestTreeItem.createFromSuite(subSuite, suiteItem)); + suiteItem.children.push(TestTreeItem.createFromSuite(subSuite, suiteItem)); }); suite.functions.forEach((fn: TestFunction) => { - suiteItem.children.push(PythonTestTreeItem.createFromFunction(fn, suiteItem)); + suiteItem.children.push(TestTreeItem.createFromFunction(fn, suiteItem)); }); return suiteItem; @@ -116,12 +116,12 @@ export class PythonTestTreeItem extends TreeItem { public static createFromFunction( fn: TestFunction, - parent: PythonTestTreeItem - ): PythonTestTreeItem { + parent: TestTreeItem + ): TestTreeItem { // tslint:disable-next-line:no-unnecessary-local-variable - const funcItem = new PythonTestTreeItem( - PythonTestTreeItemType.Function, + const funcItem = new TestTreeItem( + TestTreeItemType.Function, parent, undefined, fn.nameToRun, @@ -133,11 +133,11 @@ export class PythonTestTreeItem extends TreeItem { return funcItem; } - public get children(): PythonTestTreeItem[] { + public get children(): TestTreeItem[] { return this.myChildren; } - public get parent(): PythonTestTreeItem { + public get parent(): TestTreeItem { return this.myParent; } } diff --git a/src/client/providers/testTreeViewProvider.ts b/src/client/providers/testTreeViewProvider.ts index 7d2d59d436c0..ada8cbad3013 100644 --- a/src/client/providers/testTreeViewProvider.ts +++ b/src/client/providers/testTreeViewProvider.ts @@ -14,20 +14,20 @@ import { ITestCollectionStorageService, TestFolder, Tests, TestStatus } from '../unittests/common/types'; import { - PythonTestTreeItem, PythonTestTreeItemType + TestTreeItem, TestTreeItemType } from './testTreeViewItem'; -import { IPythonTestTreeViewProvider } from './types'; +import { ITestTreeViewProvider as ITestTreeViewProvider } from './types'; @injectable() -export class PythonTestTreeViewProvider implements IPythonTestTreeViewProvider, IDisposable { +export class TestTreeViewProvider implements ITestTreeViewProvider, IDisposable { /** * This will trigger the view to update the changed element/root and its children recursively (if shown). * To signal that root has changed, do not pass any argument or pass `undefined` or `null`. */ - public readonly onDidChangeTreeData: Event; + public readonly onDidChangeTreeData: Event; - private _onDidChangeTreeData: EventEmitter = new EventEmitter(); - private root: PythonTestTreeItem[]; + private _onDidChangeTreeData: EventEmitter = new EventEmitter(); + private root: TestTreeItem[]; private disposables: IDisposable[] = []; constructor( @@ -36,7 +36,7 @@ export class PythonTestTreeViewProvider implements IPythonTestTreeViewProvider, @inject(IDisposableRegistry) disposableRegistry: IDisposableRegistry ) { this.onDidChangeTreeData = this._onDidChangeTreeData.event; - this.root = [new PythonTestTreeItem(PythonTestTreeItemType.Root, undefined, undefined, '*', 'no tests discovered yet', TestStatus.Unknown, undefined)]; + this.root = [new TestTreeItem(TestTreeItemType.Root, undefined, undefined, '*', 'no tests discovered yet', TestStatus.Unknown, undefined)]; this.refresh(this.workspace.workspaceFolders[0].uri); disposableRegistry.push(this); this.disposables.push(this.testStore.onTestStoreUpdated(this.onTestStoreUpdated, this)); @@ -54,7 +54,7 @@ export class PythonTestTreeViewProvider implements IPythonTestTreeViewProvider, * @param element The element for which [TreeItem](#TreeItem) representation is asked for. * @return [TreeItem](#TreeItem) representation of the element */ - public async getTreeItem(element: PythonTestTreeItem): Promise { + public async getTreeItem(element: TestTreeItem): Promise { return element; } @@ -64,7 +64,7 @@ export class PythonTestTreeViewProvider implements IPythonTestTreeViewProvider, * @param element The element from which the provider gets children. Can be `undefined`. * @return Children of `element` or root if no element is passed. */ - public getChildren(element?: PythonTestTreeItem): ProviderResult { + public getChildren(element?: TestTreeItem): ProviderResult { if (element === undefined) { return this.root; } @@ -80,7 +80,7 @@ export class PythonTestTreeViewProvider implements IPythonTestTreeViewProvider, * @param element The element for which the parent has to be returned. * @return Parent of `element`. */ - public getParent?(element: PythonTestTreeItem): ProviderResult { + public getParent?(element: TestTreeItem): ProviderResult { return element.parent; } @@ -94,9 +94,9 @@ export class PythonTestTreeViewProvider implements IPythonTestTreeViewProvider, tests = this.testStore.getTests(resource); } if (tests && tests.testFolders) { - const newRoot: PythonTestTreeItem[] = []; + const newRoot: TestTreeItem[] = []; tests.testFolders.forEach((tf: TestFolder) => { - newRoot.push(PythonTestTreeItem.createFromFolder(tf)); + newRoot.push(TestTreeItem.createFromFolder(tf)); }); this.root = newRoot; this._onDidChangeTreeData.fire(); diff --git a/src/client/providers/types.ts b/src/client/providers/types.ts index 1693e71fa50c..0b7a4e8e1e01 100644 --- a/src/client/providers/types.ts +++ b/src/client/providers/types.ts @@ -7,7 +7,7 @@ import { CancellationToken, Event, ProviderResult, TreeDataProvider, Uri, WorkspaceEdit } from 'vscode'; -import { PythonTestTreeItem } from './testTreeViewItem'; +import { TestTreeItem } from './testTreeViewItem'; export const ISortImportsEditingProvider = Symbol('ISortImportsEditingProvider'); export interface ISortImportsEditingProvider { @@ -16,9 +16,9 @@ export interface ISortImportsEditingProvider { registerCommands(): void; } -export const IPythonTestTreeViewProvider = Symbol('IPythonTestTreeViewProvider'); -export interface IPythonTestTreeViewProvider extends TreeDataProvider { - onDidChangeTreeData: Event; - getTreeItem(element: PythonTestTreeItem): Promise; - getChildren(element?: PythonTestTreeItem): ProviderResult; +export const ITestTreeViewProvider = Symbol('ITestTreeViewProvider'); +export interface ITestTreeViewProvider extends TreeDataProvider { + onDidChangeTreeData: Event; + getTreeItem(element: TestTreeItem): Promise; + getChildren(element?: TestTreeItem): ProviderResult; } From 1393fbb4febdb9d48bd08e128671f32266462c86 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 6 Feb 2019 14:07:43 -0800 Subject: [PATCH 5/8] PR suggestions - simplify name of event for test store updates - --- src/client/providers/testTreeViewProvider.ts | 2 +- src/client/unittests/common/services/storageService.ts | 4 ++-- src/client/unittests/common/types.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/client/providers/testTreeViewProvider.ts b/src/client/providers/testTreeViewProvider.ts index ada8cbad3013..eced3740e77d 100644 --- a/src/client/providers/testTreeViewProvider.ts +++ b/src/client/providers/testTreeViewProvider.ts @@ -39,7 +39,7 @@ export class TestTreeViewProvider implements ITestTreeViewProvider, IDisposable this.root = [new TestTreeItem(TestTreeItemType.Root, undefined, undefined, '*', 'no tests discovered yet', TestStatus.Unknown, undefined)]; this.refresh(this.workspace.workspaceFolders[0].uri); disposableRegistry.push(this); - this.disposables.push(this.testStore.onTestStoreUpdated(this.onTestStoreUpdated, this)); + this.disposables.push(this.testStore.onUpdated(this.onTestStoreUpdated, this)); } // tslint:disable-next-line:no-empty diff --git a/src/client/unittests/common/services/storageService.ts b/src/client/unittests/common/services/storageService.ts index 685c729095a3..a9602a4908a4 100644 --- a/src/client/unittests/common/services/storageService.ts +++ b/src/client/unittests/common/services/storageService.ts @@ -8,14 +8,14 @@ import { FlattenedTestFunction, FlattenedTestSuite, ITestCollectionStorageServic @injectable() export class TestCollectionStorageService implements ITestCollectionStorageService { - public readonly onTestStoreUpdated: Event; + public readonly onUpdated: Event; private testsIndexedByWorkspaceUri = new Map(); private _onTestStoreUpdated: EventEmitter = new EventEmitter(); constructor(@inject(IDisposableRegistry) disposables: Disposable[]) { disposables.push(this); - this.onTestStoreUpdated = this._onTestStoreUpdated.event; + this.onUpdated = this._onTestStoreUpdated.event; } public getTests(wkspace: Uri): Tests | undefined { const workspaceFolder = this.getWorkspaceFolderPath(wkspace) || ''; diff --git a/src/client/unittests/common/types.ts b/src/client/unittests/common/types.ts index 8200d0a00b2c..dab75d0f160c 100644 --- a/src/client/unittests/common/types.ts +++ b/src/client/unittests/common/types.ts @@ -184,7 +184,7 @@ export interface ITestVisitor { export const ITestCollectionStorageService = Symbol('ITestCollectionStorageService'); export interface ITestCollectionStorageService extends Disposable { - onTestStoreUpdated: Event; + onUpdated: Event; getTests(wkspace: Uri): Tests | undefined; storeTests(wkspace: Uri, tests: Tests | null | undefined): void; findFlattendTestFunction(resource: Uri, func: TestFunction): FlattenedTestFunction | undefined; From 7dfaabe993434820193dbb1874d2c11084f74180 Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 6 Feb 2019 14:56:10 -0800 Subject: [PATCH 6/8] PR Suggestions - move TreeView files to more appropriate location - move registrations to more appropriate location - clean up some lengthy import statements --- src/client/extension.ts | 7 +-- src/client/providers/serviceRegistry.ts | 6 +-- src/client/providers/types.ts | 2 +- src/client/unittests/main.ts | 44 +++++++++++++++---- .../providers/testTreeViewItem.ts | 2 +- .../providers/testTreeViewProvider.ts | 12 ++--- src/client/unittests/serviceRegistry.ts | 17 +++++-- 7 files changed, 60 insertions(+), 30 deletions(-) rename src/client/{ => unittests}/providers/testTreeViewItem.ts (99%) rename src/client/{ => unittests}/providers/testTreeViewProvider.ts (92%) diff --git a/src/client/extension.ts b/src/client/extension.ts index 41811311c882..5e59846aaa58 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -93,9 +93,7 @@ import { ReplProvider } from './providers/replProvider'; import { registerTypes as providersRegisterTypes } from './providers/serviceRegistry'; import { activateSimplePythonRefactorProvider } from './providers/simpleRefactorProvider'; import { TerminalProvider } from './providers/terminalProvider'; -import { - IPythonTestTreeViewProvider, ISortImportsEditingProvider -} from './providers/types'; +import { ISortImportsEditingProvider } from './providers/types'; import { activateUpdateSparkLibraryProvider } from './providers/updateSparkLibraryProvider'; import { sendTelemetryEvent } from './telemetry'; import { EventName } from './telemetry/constants'; @@ -209,9 +207,6 @@ async function activateUnsafe(context: ExtensionContext): Promise context.subscriptions.push(languages.registerCodeActionsProvider(PYTHON, new PythonCodeActionProvider(), { providedCodeActionKinds: [CodeActionKind.SourceOrganizeImports] })); - const testViewProvider = serviceContainer.get(IPythonTestTreeViewProvider); - context.subscriptions.push(window.registerTreeDataProvider('python_tests', testViewProvider)); - serviceContainer.getAll(IDebugConfigurationService).forEach(debugConfigProvider => { context.subscriptions.push(debug.registerDebugConfigurationProvider(DebuggerTypeName, debugConfigProvider)); }); diff --git a/src/client/providers/serviceRegistry.ts b/src/client/providers/serviceRegistry.ts index 2c5b49ee4304..7418e0175e51 100644 --- a/src/client/providers/serviceRegistry.ts +++ b/src/client/providers/serviceRegistry.ts @@ -5,12 +5,8 @@ import { IServiceManager } from '../ioc/types'; import { SortImportsEditingProvider } from './importSortProvider'; -import { TestTreeViewProvider } from './testTreeViewProvider'; -import { - ISortImportsEditingProvider, ITestTreeViewProvider -} from './types'; +import { ISortImportsEditingProvider } from './types'; export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(ISortImportsEditingProvider, SortImportsEditingProvider); - serviceManager.addSingleton(ITestTreeViewProvider, TestTreeViewProvider); } diff --git a/src/client/providers/types.ts b/src/client/providers/types.ts index 0b7a4e8e1e01..8784e4fdf18d 100644 --- a/src/client/providers/types.ts +++ b/src/client/providers/types.ts @@ -7,7 +7,7 @@ import { CancellationToken, Event, ProviderResult, TreeDataProvider, Uri, WorkspaceEdit } from 'vscode'; -import { TestTreeItem } from './testTreeViewItem'; +import { TestTreeItem } from '../unittests/providers/testTreeViewItem'; export const ISortImportsEditingProvider = Symbol('ISortImportsEditingProvider'); export interface ISortImportsEditingProvider { diff --git a/src/client/unittests/main.ts b/src/client/unittests/main.ts index 9f0ca5d71ec8..8402ac763dc1 100644 --- a/src/client/unittests/main.ts +++ b/src/client/unittests/main.ts @@ -3,24 +3,42 @@ // tslint:disable:no-duplicate-imports no-unnecessary-callback-wrapper import { inject, injectable } from 'inversify'; -import { ConfigurationChangeEvent, Disposable, OutputChannel, TextDocument, Uri } from 'vscode'; -import * as vscode from 'vscode'; -import { ICommandManager, IDocumentManager, IWorkspaceService } from '../common/application/types'; +import { + ConfigurationChangeEvent, Disposable, + DocumentSymbolProvider, EventEmitter, + OutputChannel, TextDocument, Uri, window +} from 'vscode'; +import { + ICommandManager, IDocumentManager, IWorkspaceService +} from '../common/application/types'; import * as constants from '../common/constants'; import '../common/extensions'; -import { IConfigurationService, IDisposableRegistry, ILogger, IOutputChannel } from '../common/types'; +import { + IConfigurationService, IDisposableRegistry, + ILogger, IOutputChannel +} from '../common/types'; import { IServiceContainer } from '../ioc/types'; +import { ITestTreeViewProvider } from '../providers/types'; import { EventName } from '../telemetry/constants'; import { sendTelemetryEvent } from '../telemetry/index'; import { activateCodeLenses } from './codeLenses/main'; -import { CANCELLATION_REASON, CommandSource, TEST_OUTPUT_CHANNEL } from './common/constants'; +import { + CANCELLATION_REASON, CommandSource, TEST_OUTPUT_CHANNEL +} from './common/constants'; import { selectTestWorkspace } from './common/testUtils'; -import { ITestCollectionStorageService, ITestManager, IWorkspaceTestManagerService, TestFile, TestFunction, TestStatus, TestsToRun } from './common/types'; -import { ITestDisplay, ITestResultDisplay, IUnitTestConfigurationService, IUnitTestManagementService } from './types'; +import { + ITestCollectionStorageService, ITestManager, + IWorkspaceTestManagerService, TestFile, + TestFunction, TestStatus, TestsToRun +} from './common/types'; +import { + ITestDisplay, ITestResultDisplay, + IUnitTestConfigurationService, IUnitTestManagementService +} from './types'; @injectable() export class UnitTestManagementService implements IUnitTestManagementService, Disposable { - private readonly outputChannel: vscode.OutputChannel; + private readonly outputChannel: OutputChannel; private readonly disposableRegistry: Disposable[]; private workspaceTestManagerService?: IWorkspaceTestManagerService; private documentManager: IDocumentManager; @@ -28,7 +46,7 @@ export class UnitTestManagementService implements IUnitTestManagementService, Di private testResultDisplay?: ITestResultDisplay; private autoDiscoverTimer?: NodeJS.Timer; private configChangedTimer?: NodeJS.Timer; - private readonly onDidChange: vscode.EventEmitter = new vscode.EventEmitter(); + private readonly onDidChange: EventEmitter = new EventEmitter(); constructor(@inject(IServiceContainer) private serviceContainer: IServiceContainer) { this.disposableRegistry = serviceContainer.get(IDisposableRegistry); @@ -45,12 +63,20 @@ export class UnitTestManagementService implements IUnitTestManagementService, Di } public async activate(symboldProvider: vscode.DocumentSymbolProvider): Promise { this.workspaceTestManagerService = this.serviceContainer.get(IWorkspaceTestManagerService); + const disposablesRegistry = this.serviceContainer.get(IDisposableRegistry); this.registerHandlers(); this.registerCommands(); + + // register provider... + const testViewProvider = this.serviceContainer.get(ITestTreeViewProvider); + const disposable = window.registerTreeDataProvider('python_tests', testViewProvider); + disposablesRegistry.push(disposable); + this.autoDiscoverTests() .catch(ex => this.serviceContainer.get(ILogger).logError('Failed to auto discover tests upon activation', ex)); await this.registerSymbolProvider(symboldProvider); + public async activateCodeLenses(symboldProvider: DocumentSymbolProvider): Promise { } public async getTestManager(displayTestNotConfiguredMessage: boolean, resource?: Uri): Promise { let wkspace: Uri | undefined; diff --git a/src/client/providers/testTreeViewItem.ts b/src/client/unittests/providers/testTreeViewItem.ts similarity index 99% rename from src/client/providers/testTreeViewItem.ts rename to src/client/unittests/providers/testTreeViewItem.ts index 1107244f831c..9e2ce267ec9c 100644 --- a/src/client/providers/testTreeViewItem.ts +++ b/src/client/unittests/providers/testTreeViewItem.ts @@ -9,7 +9,7 @@ import { import { TestFile, TestFolder, TestFunction, TestStatus, TestSuite -} from '../unittests/common/types'; +} from '../common/types'; export enum TestTreeItemType { Root = 'Root', diff --git a/src/client/providers/testTreeViewProvider.ts b/src/client/unittests/providers/testTreeViewProvider.ts similarity index 92% rename from src/client/providers/testTreeViewProvider.ts rename to src/client/unittests/providers/testTreeViewProvider.ts index eced3740e77d..4013d4bd7b34 100644 --- a/src/client/providers/testTreeViewProvider.ts +++ b/src/client/unittests/providers/testTreeViewProvider.ts @@ -7,16 +7,18 @@ import { inject, injectable } from 'inversify'; import { Event, EventEmitter, ProviderResult, Uri } from 'vscode'; -import { IWorkspaceService } from '../common/application/types'; -import { traceDecorators } from '../common/logger'; -import { IDisposable, IDisposableRegistry, Resource } from '../common/types'; +import { IWorkspaceService } from '../../common/application/types'; +import { traceDecorators } from '../../common/logger'; +import { + IDisposable, IDisposableRegistry, Resource +} from '../../common/types'; +import { ITestTreeViewProvider } from '../../providers/types'; import { ITestCollectionStorageService, TestFolder, Tests, TestStatus -} from '../unittests/common/types'; +} from '../common/types'; import { TestTreeItem, TestTreeItemType } from './testTreeViewItem'; -import { ITestTreeViewProvider as ITestTreeViewProvider } from './types'; @injectable() export class TestTreeViewProvider implements ITestTreeViewProvider, IDisposable { diff --git a/src/client/unittests/serviceRegistry.ts b/src/client/unittests/serviceRegistry.ts index d5007cfed20a..07179353295d 100644 --- a/src/client/unittests/serviceRegistry.ts +++ b/src/client/unittests/serviceRegistry.ts @@ -3,6 +3,7 @@ import { Uri } from 'vscode'; import { IServiceContainer, IServiceManager } from '../ioc/types'; +import { ITestTreeViewProvider } from '../providers/types'; import { ArgumentsHelper } from './common/argumentsHelper'; import { NOSETEST_PROVIDER, PYTEST_PROVIDER, UNITTEST_PROVIDER } from './common/constants'; import { DebugLauncher } from './common/debugLauncher'; @@ -18,8 +19,11 @@ import { TestFlatteningVisitor } from './common/testVisitors/flatteningVisitor'; import { TestFolderGenerationVisitor } from './common/testVisitors/folderGenerationVisitor'; import { TestResultResetVisitor } from './common/testVisitors/resultResetVisitor'; import { - ITestCollectionStorageService, ITestConfigSettingsService, ITestDebugLauncher, ITestDiscoveryService, ITestManager, ITestManagerFactory, ITestManagerService, ITestManagerServiceFactory, - ITestMessageService, ITestResultsService, ITestRunner, ITestsHelper, ITestsParser, ITestVisitor, IUnitTestSocketServer, IWorkspaceTestManagerService, IXUnitParser, TestProvider + ITestCollectionStorageService, ITestConfigSettingsService, ITestDebugLauncher, + ITestDiscoveryService, ITestManager, ITestManagerFactory, ITestManagerService, + ITestManagerServiceFactory, ITestMessageService, ITestResultsService, ITestRunner, + ITestsHelper, ITestsParser, ITestVisitor, IUnitTestSocketServer, IWorkspaceTestManagerService, + IXUnitParser, TestProvider } from './common/types'; import { XUnitParser } from './common/xUnitParser'; import { UnitTestConfigurationService } from './configuration'; @@ -33,13 +37,19 @@ import { TestManagerRunner as NoseTestManagerRunner } from './nosetest/runner'; import { ArgumentsService as NoseTestArgumentsService } from './nosetest/services/argsService'; import { TestDiscoveryService as NoseTestDiscoveryService } from './nosetest/services/discoveryService'; import { TestsParser as NoseTestTestsParser } from './nosetest/services/parserService'; +import { TestTreeViewProvider } from './providers/testTreeViewProvider'; import { TestManager as PyTestTestManager } from './pytest/main'; import { TestManagerRunner as PytestManagerRunner } from './pytest/runner'; import { ArgumentsService as PyTestArgumentsService } from './pytest/services/argsService'; import { TestDiscoveryService as PytestTestDiscoveryService } from './pytest/services/discoveryService'; import { TestsParser as PytestTestsParser } from './pytest/services/parserService'; import { TestMessageService } from './pytest/services/testMessageService'; -import { IArgumentsHelper, IArgumentsService, ITestConfigurationManagerFactory, ITestDisplay, ITestManagerRunner, ITestResultDisplay, IUnitTestConfigurationService, IUnitTestDiagnosticService, IUnitTestHelper, IUnitTestManagementService } from './types'; +import { + IArgumentsHelper, IArgumentsService, ITestConfigurationManagerFactory, + ITestDisplay, ITestManagerRunner, ITestResultDisplay, + IUnitTestConfigurationService, IUnitTestDiagnosticService, + IUnitTestHelper, IUnitTestManagementService +} from './types'; import { UnitTestHelper } from './unittest/helper'; import { TestManager as UnitTestTestManager } from './unittest/main'; import { TestManagerRunner as UnitTestTestManagerRunner } from './unittest/runner'; @@ -92,6 +102,7 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IUnitTestDiagnosticService, UnitTestDiagnosticService); serviceManager.addSingleton(ITestMessageService, TestMessageService, PYTEST_PROVIDER); + serviceManager.addSingleton(ITestTreeViewProvider, TestTreeViewProvider); serviceManager.addFactory(ITestManagerFactory, (context) => { return (testProvider: TestProvider, workspaceFolder: Uri, rootDirectory: string) => { From f30733d48f52eb859f418d4cad9700f2a210c02a Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 6 Feb 2019 15:19:15 -0800 Subject: [PATCH 7/8] argh hygiene --- src/client/unittests/main.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/client/unittests/main.ts b/src/client/unittests/main.ts index 8402ac763dc1..367256ca0f76 100644 --- a/src/client/unittests/main.ts +++ b/src/client/unittests/main.ts @@ -61,7 +61,7 @@ export class UnitTestManagementService implements IUnitTestManagementService, Di this.workspaceTestManagerService.dispose(); } } - public async activate(symboldProvider: vscode.DocumentSymbolProvider): Promise { + public async activate(symbolProvider: DocumentSymbolProvider): Promise { this.workspaceTestManagerService = this.serviceContainer.get(IWorkspaceTestManagerService); const disposablesRegistry = this.serviceContainer.get(IDisposableRegistry); @@ -75,9 +75,14 @@ export class UnitTestManagementService implements IUnitTestManagementService, Di this.autoDiscoverTests() .catch(ex => this.serviceContainer.get(ILogger).logError('Failed to auto discover tests upon activation', ex)); - await this.registerSymbolProvider(symboldProvider); - public async activateCodeLenses(symboldProvider: DocumentSymbolProvider): Promise { + await this.registerSymbolProvider(symbolProvider); } + + public async activateCodeLenses(symbolProvider: DocumentSymbolProvider): Promise { + const testCollectionStorage = this.serviceContainer.get(ITestCollectionStorageService); + this.disposableRegistry.push(activateCodeLenses(this.onDidChange, symbolProvider, testCollectionStorage)); + } + public async getTestManager(displayTestNotConfiguredMessage: boolean, resource?: Uri): Promise { let wkspace: Uri | undefined; if (resource) { @@ -304,9 +309,9 @@ export class UnitTestManagementService implements IUnitTestManagementService, Di this.testResultDisplay.displayProgressStatus(promise, debug); await promise; } - private async registerSymbolProvider(symboldProvider: vscode.DocumentSymbolProvider): Promise { + private async registerSymbolProvider(symbolProvider: DocumentSymbolProvider): Promise { const testCollectionStorage = this.serviceContainer.get(ITestCollectionStorageService); - this.disposableRegistry.push(activateCodeLenses(this.onDidChange, symboldProvider, testCollectionStorage)); + this.disposableRegistry.push(activateCodeLenses(this.onDidChange, symbolProvider, testCollectionStorage)); } private registerCommands(): void { const disposablesRegistry = this.serviceContainer.get(IDisposableRegistry); From 9fcdfbf04978c7b00c52aeaa13bdbeb83f2f0fcf Mon Sep 17 00:00:00 2001 From: Derek Keeler Date: Wed, 6 Feb 2019 16:45:01 -0800 Subject: [PATCH 8/8] Include TreeItem from vscodeMocks in unit test setup --- src/test/vscode-mock.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/vscode-mock.ts b/src/test/vscode-mock.ts index 33032c9ab3bd..77d1015fe97c 100644 --- a/src/test/vscode-mock.ts +++ b/src/test/vscode-mock.ts @@ -33,7 +33,7 @@ export function initialize() { generateMock('scm'); // When upgrading to npm 9-10, this might have to change, as we could have explicit imports (named imports). - Module._load = function(request: any, _parent: any) { + Module._load = function (request: any, _parent: any) { if (request === 'vscode') { return mockedVSCode; } @@ -71,6 +71,7 @@ mockedVSCode.RelativePattern = vscodeMocks.vscMockExtHostedTypes.RelativePattern mockedVSCode.ProgressLocation = vscodeMocks.vscMockExtHostedTypes.ProgressLocation; mockedVSCode.ViewColumn = vscodeMocks.vscMockExtHostedTypes.ViewColumn; mockedVSCode.TextEditorRevealType = vscodeMocks.vscMockExtHostedTypes.TextEditorRevealType; +mockedVSCode.TreeItem = vscodeMocks.vscMockExtHostedTypes.TreeItem; // This API is used in src/client/telemetry/telemetry.ts const extensions = TypeMoq.Mock.ofType();