From 5ba2e56210ff516791eb979fa719f50c10d7e014 Mon Sep 17 00:00:00 2001 From: mattseddon <37993418+mattseddon@users.noreply.github.com> Date: Wed, 4 Aug 2021 03:16:23 +1000 Subject: [PATCH] Switch `ExperimentsParamsAndMetricsTree` to string | ParamsAndMetricsItem tree type (#700) * move params and metrics tree to mixed type * remove get param or metric methods where possible * rework tree data type * improve test * improve test variable names --- extension/src/experiments/index.ts | 4 - .../src/experiments/paramsAndMetrics/model.ts | 35 ++- .../experiments/paramsAndMetrics/tree.test.ts | 250 +++++++++++------ .../src/experiments/paramsAndMetrics/tree.ts | 114 ++++---- extension/src/experiments/repository.ts | 4 - extension/src/resourceLocator.ts | 2 +- .../experiments/paramsAndMetrics/tree.test.ts | 264 ++++++++---------- 7 files changed, 360 insertions(+), 313 deletions(-) diff --git a/extension/src/experiments/index.ts b/extension/src/experiments/index.ts index ed7987dcd6..b500e6d004 100644 --- a/extension/src/experiments/index.ts +++ b/extension/src/experiments/index.ts @@ -83,10 +83,6 @@ export class Experiments { return Object.keys(this.experiments) } - public getParamOrMetric(dvcRoot: string, path: string) { - return this.getRepository(dvcRoot).getParamOrMetric(path) - } - public getChildParamsOrMetrics(dvcRoot: string, path: string) { return this.getRepository(dvcRoot).getChildParamsOrMetrics(path) } diff --git a/extension/src/experiments/paramsAndMetrics/model.ts b/extension/src/experiments/paramsAndMetrics/model.ts index 2667c0560a..3bd3366673 100644 --- a/extension/src/experiments/paramsAndMetrics/model.ts +++ b/extension/src/experiments/paramsAndMetrics/model.ts @@ -45,25 +45,20 @@ export class ParamsAndMetricsModel { return this.data.filter(paramOrMetric => !paramOrMetric.hasChildren) } - public getParamOrMetric(path: string) { - const paramOrMetric = this.data?.find( - paramOrMetric => paramOrMetric.path === path - ) - if (paramOrMetric) { - return { - ...paramOrMetric, - descendantStatuses: this.getDescendantsStatuses(paramOrMetric.path), - status: this.status[paramOrMetric.path] - } - } - } - public getChildren(path: string) { - return this.data?.filter(paramOrMetric => - path - ? paramOrMetric.parentPath === path - : ['metrics', 'params'].includes(paramOrMetric.parentPath) - ) + return this.data + ?.filter(paramOrMetric => + path + ? paramOrMetric.parentPath === path + : ['metrics', 'params'].includes(paramOrMetric.parentPath) + ) + .map(paramOrMetric => { + return { + ...paramOrMetric, + descendantStatuses: this.getDescendantsStatuses(paramOrMetric.path), + status: this.status[paramOrMetric.path] + } + }) } public toggleStatus(path: string) { @@ -90,6 +85,10 @@ export class ParamsAndMetricsModel { }) } + private getParamOrMetric(path: string) { + return this.data?.find(paramOrMetric => paramOrMetric.path === path) + } + private setAreParentsSelected(path: string) { const changed = this.getParamOrMetric(path) if (!changed) { diff --git a/extension/src/experiments/paramsAndMetrics/tree.test.ts b/extension/src/experiments/paramsAndMetrics/tree.test.ts index 006821294e..6f201cf197 100644 --- a/extension/src/experiments/paramsAndMetrics/tree.test.ts +++ b/extension/src/experiments/paramsAndMetrics/tree.test.ts @@ -1,12 +1,18 @@ import { join } from 'path' import { Disposable, Disposer } from '@hediet/std/disposable' import { mocked } from 'ts-jest/utils' -import { commands, EventEmitter, TreeItem, Uri, window } from 'vscode' +import { + commands, + EventEmitter, + TreeItem, + TreeItemCollapsibleState, + Uri, + window +} from 'vscode' import { ExperimentsParamsAndMetricsTree } from './tree' import complexColumnData from '../webview/complex-column-example.json' -import { ResourceLocator } from '../../resourceLocator' +import { Resource, ResourceLocator } from '../../resourceLocator' import { Experiments } from '..' -import { ParamOrMetric } from '../webview/contract' import { Status } from '../paramsAndMetrics/model' const mockedCommands = mocked(commands) @@ -25,12 +31,10 @@ const mockedTreeItem = mocked(TreeItem) const mockedDisposable = mocked(Disposable) const mockedGetChildParamsOrMetrics = jest.fn() -const mockedGetParamOrMetric = jest.fn() const mockedGetDvcRoots = jest.fn() const mockedExperiments = { getChildParamsOrMetrics: mockedGetChildParamsOrMetrics, getDvcRoots: mockedGetDvcRoots, - getParamOrMetric: mockedGetParamOrMetric, isReady: () => true, paramsOrMetricsChanged: mockedParamsOrMetricsChanged } as unknown as Experiments @@ -38,15 +42,15 @@ const mockedExperiments = { const mockedSelectedCheckbox = { dark: join('path', 'to', 'checkbox-c.svg'), light: join('path', 'to', 'checkbox-c.svg') -} +} as unknown as Resource const mockedIndeterminateCheckbox = { dark: join('path', 'to', 'checkbox-i.svg'), light: join('path', 'to', 'checkbox-i.svg') -} +} as unknown as Resource const mockedEmptyCheckbox = { dark: join('path', 'to', 'checkbox-e.svg'), light: join('path', 'to', 'checkbox-e.svg') -} +} as unknown as Resource const mockedResourceLocator = { checkedCheckbox: mockedSelectedCheckbox, emptyCheckbox: mockedEmptyCheckbox, @@ -67,6 +71,16 @@ beforeEach(() => { }) describe('ExperimentsParamsAndMetricsTree', () => { + const rootParamsAndMetrics = complexColumnData + .filter(paramOrMetric => + ['metrics', 'params'].includes(paramOrMetric.parentPath) + ) + .map(paramOrMetric => ({ + ...paramOrMetric, + descendantStatuses: [], + status: Status.selected + })) + describe('getChildren', () => { it('should return the experiments roots if there are multiple repositories and no path is provided', async () => { const experimentsParamsAndMetricsTree = @@ -95,17 +109,25 @@ describe('ExperimentsParamsAndMetricsTree', () => { const mockedDvcRoot = join('path', 'to', 'only', 'root') mockedGetDvcRoots.mockReturnValueOnce([mockedDvcRoot]) - mockedGetChildParamsOrMetrics.mockReturnValueOnce( - complexColumnData.filter(paramOrMetric => - ['metrics', 'params'].includes(paramOrMetric.parentPath) - ) - ) + mockedGetChildParamsOrMetrics.mockReturnValueOnce(rootParamsAndMetrics) const children = await experimentsParamsAndMetricsTree.getChildren() expect(children).toEqual([ - join(mockedDvcRoot, 'params', 'params.yaml'), - join(mockedDvcRoot, 'metrics', 'summary.json') + { + collapsibleState: 1, + description: undefined, + dvcRoot: mockedDvcRoot, + iconPath: mockedSelectedCheckbox, + path: join('params', 'params.yaml') + }, + { + collapsibleState: 1, + description: undefined, + dvcRoot: mockedDvcRoot, + iconPath: mockedSelectedCheckbox, + path: join('metrics', 'summary.json') + } ]) }) @@ -119,62 +141,144 @@ describe('ExperimentsParamsAndMetricsTree', () => { const mockedDvcRoot = join('path', 'to', 'dvc', 'repo') mockedGetDvcRoots.mockReturnValueOnce([mockedDvcRoot]) + mockedGetChildParamsOrMetrics.mockReturnValueOnce(rootParamsAndMetrics) + await experimentsParamsAndMetricsTree.getChildren() - mockedGetChildParamsOrMetrics.mockReturnValueOnce( - complexColumnData.filter(paramOrMetric => - ['metrics', 'params'].includes(paramOrMetric.parentPath) - ) - ) + mockedGetChildParamsOrMetrics.mockReturnValueOnce(rootParamsAndMetrics) const children = await experimentsParamsAndMetricsTree.getChildren( mockedDvcRoot ) - const relParamsPath = join('params', 'params.yaml') - const paramsPath = join(mockedDvcRoot, relParamsPath) + + const paramsPath = join('params', 'params.yaml') + const processPath = join(paramsPath, 'process') expect(children).toEqual([ - paramsPath, - join(mockedDvcRoot, 'metrics', 'summary.json') + { + collapsibleState: 1, + description: undefined, + dvcRoot: mockedDvcRoot, + iconPath: mockedSelectedCheckbox, + path: paramsPath + }, + { + collapsibleState: 1, + description: undefined, + dvcRoot: mockedDvcRoot, + iconPath: mockedSelectedCheckbox, + path: join('metrics', 'summary.json') + } ]) mockedGetChildParamsOrMetrics.mockReturnValueOnce( - complexColumnData.filter( - paramOrMetric => relParamsPath === paramOrMetric.parentPath - ) - ) - const grandChildren = await experimentsParamsAndMetricsTree.getChildren( - paramsPath + complexColumnData + .filter(paramOrMetric => paramsPath === paramOrMetric.parentPath) + .map(param => { + if (param.path === processPath) { + return { + ...param, + descendantStatuses: [Status.unselected, Status.selected], + hasChildren: true, + status: Status.indeterminate + } + } + return { + ...param, + descendantStatuses: undefined, + hasChildren: false, + status: Status.selected + } + }) ) - const relParamsProcessPath = join(relParamsPath, 'process') - const paramsProcessPath = join(mockedDvcRoot, relParamsProcessPath) + const grandChildren = await experimentsParamsAndMetricsTree.getChildren({ + collapsibleState: TreeItemCollapsibleState.Collapsed, + description: undefined, + dvcRoot: mockedDvcRoot, + iconPath: mockedSelectedCheckbox, + path: paramsPath + }) expect(grandChildren).toEqual([ - join(paramsPath, 'epochs'), - join(paramsPath, 'learning_rate'), - join(paramsPath, 'dvc_logs_dir'), - join(paramsPath, 'log_file'), - join(paramsPath, 'dropout'), - paramsProcessPath + { + collapsibleState: 0, + dvcRoot: mockedDvcRoot, + iconPath: mockedSelectedCheckbox, + path: join(paramsPath, 'epochs') + }, + { + collapsibleState: 0, + dvcRoot: mockedDvcRoot, + iconPath: mockedSelectedCheckbox, + path: join(paramsPath, 'learning_rate') + }, + { + collapsibleState: 0, + dvcRoot: mockedDvcRoot, + iconPath: mockedSelectedCheckbox, + path: join(paramsPath, 'dvc_logs_dir') + }, + { + collapsibleState: 0, + dvcRoot: mockedDvcRoot, + iconPath: mockedSelectedCheckbox, + path: join(paramsPath, 'log_file') + }, + { + collapsibleState: 0, + dvcRoot: mockedDvcRoot, + iconPath: mockedSelectedCheckbox, + path: join(paramsPath, 'dropout') + }, + { + collapsibleState: 1, + description: '1/2', + dvcRoot: mockedDvcRoot, + iconPath: mockedIndeterminateCheckbox, + path: processPath + } ]) mockedGetChildParamsOrMetrics.mockReturnValueOnce( - complexColumnData.filter( - paramOrMetric => relParamsProcessPath === paramOrMetric.parentPath - ) + complexColumnData + .filter(paramOrMetric => processPath === paramOrMetric.parentPath) + .map(param => ({ + ...param, + descendantStatuses: undefined, + hasChildren: false, + status: Status.selected + })) ) const greatGrandChildren = - await experimentsParamsAndMetricsTree.getChildren(paramsProcessPath) + await experimentsParamsAndMetricsTree.getChildren({ + collapsibleState: TreeItemCollapsibleState.Collapsed, + description: '1/2', + dvcRoot: mockedDvcRoot, + iconPath: mockedIndeterminateCheckbox, + path: processPath + }) expect(greatGrandChildren).toEqual([ - join(paramsProcessPath, 'threshold'), - join(paramsProcessPath, 'test_arg') + { + collapsibleState: 0, + description: undefined, + dvcRoot: mockedDvcRoot, + iconPath: mockedSelectedCheckbox, + path: join(processPath, 'threshold') + }, + { + collapsibleState: 0, + description: undefined, + dvcRoot: mockedDvcRoot, + iconPath: mockedSelectedCheckbox, + path: join(processPath, 'test_arg') + } ]) }) }) describe('getTreeItem', () => { - it('should return the correct tree item for a repository root', async () => { + it('should return the correct tree item for a repository root', () => { let mockedItem = {} mockedTreeItem.mockImplementationOnce(function (uri, collapsibleState) { expect(collapsibleState).toEqual(1) @@ -190,10 +294,6 @@ describe('ExperimentsParamsAndMetricsTree', () => { const mockedDvcRoot = join('dvc', 'repo') - mockedGetDvcRoots.mockReturnValueOnce([mockedDvcRoot]) - - await experimentsParamsAndMetricsTree.getChildren() - const treeItem = experimentsParamsAndMetricsTree.getTreeItem(mockedDvcRoot) @@ -218,31 +318,22 @@ describe('ExperimentsParamsAndMetricsTree', () => { const relParamsPath = join('params', 'params.yml') const paramsPath = join(mockedDvcRoot, relParamsPath) - jest - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .spyOn(experimentsParamsAndMetricsTree as any, 'getDetails') - .mockReturnValueOnce([mockedDvcRoot, relParamsPath]) - - mockedGetParamOrMetric.mockReturnValueOnce({ - descendantStatuses: [ - Status.selected, - Status.selected, - Status.selected, - Status.unselected - ], - hasChildren: true, - status: Status.selected - } as unknown as ParamOrMetric) + const paramsAndMetricsItem = { + collapsibleState: TreeItemCollapsibleState.Collapsed, + description: '3/4', + dvcRoot: mockedDvcRoot, + iconPath: mockedSelectedCheckbox, + path: relParamsPath + } - const treeItem = experimentsParamsAndMetricsTree.getTreeItem(paramsPath) + const treeItem = + experimentsParamsAndMetricsTree.getTreeItem(paramsAndMetricsItem) expect(mockedTreeItem).toBeCalledTimes(1) - expect(mockedGetParamOrMetric).toBeCalledTimes(1) - expect(mockedGetParamOrMetric).toBeCalledWith(mockedDvcRoot, relParamsPath) expect(treeItem).toEqual({ collapsibleState: 1, command: { - arguments: [paramsPath], + arguments: [{ dvcRoot: mockedDvcRoot, path: relParamsPath }], command: 'dvc.views.experimentsParamsAndMetricsTree.toggleStatus', title: 'toggle' }, @@ -266,25 +357,22 @@ describe('ExperimentsParamsAndMetricsTree', () => { const relParamsPath = join('params', 'params.yml') const paramsPath = join(mockedDvcRoot, relParamsPath) - jest - // eslint-disable-next-line @typescript-eslint/no-explicit-any - .spyOn(experimentsParamsAndMetricsTree as any, 'getDetails') - .mockReturnValueOnce([mockedDvcRoot, relParamsPath]) - - mockedGetParamOrMetric.mockReturnValueOnce({ - hasChildren: false, - status: Status.unselected - } as unknown as ParamOrMetric) + const paramsAndMetricsItem = { + collapsibleState: TreeItemCollapsibleState.None, + description: undefined, + dvcRoot: mockedDvcRoot, + iconPath: mockedEmptyCheckbox, + path: relParamsPath + } - const treeItem = experimentsParamsAndMetricsTree.getTreeItem(paramsPath) + const treeItem = + experimentsParamsAndMetricsTree.getTreeItem(paramsAndMetricsItem) expect(mockedTreeItem).toBeCalledTimes(1) - expect(mockedGetParamOrMetric).toBeCalledTimes(1) - expect(mockedGetParamOrMetric).toBeCalledWith(mockedDvcRoot, relParamsPath) expect(treeItem).toEqual({ collapsibleState: 0, command: { - arguments: [paramsPath], + arguments: [{ dvcRoot: mockedDvcRoot, path: relParamsPath }], command: 'dvc.views.experimentsParamsAndMetricsTree.toggleStatus', title: 'toggle' }, diff --git a/extension/src/experiments/paramsAndMetrics/tree.ts b/extension/src/experiments/paramsAndMetrics/tree.ts index 41565d728e..4e45ea4060 100644 --- a/extension/src/experiments/paramsAndMetrics/tree.ts +++ b/extension/src/experiments/paramsAndMetrics/tree.ts @@ -1,4 +1,4 @@ -import { join, relative } from 'path' +import { join } from 'path' import { Disposable } from '@hediet/std/disposable' import { commands, @@ -12,12 +12,19 @@ import { } from 'vscode' import { Status } from './model' import { Experiments } from '..' -import { ResourceLocator } from '../../resourceLocator' -import { ParamOrMetric } from '../webview/contract' +import { Resource, ResourceLocator } from '../../resourceLocator' import { definedAndNonEmpty, flatten } from '../../util/array' +type ParamsAndMetricsItem = { + description: string | undefined + dvcRoot: string + collapsibleState: TreeItemCollapsibleState + path: string + iconPath: Resource +} + export class ExperimentsParamsAndMetricsTree - implements TreeDataProvider + implements TreeDataProvider { public dispose = Disposable.fn() @@ -25,9 +32,8 @@ export class ExperimentsParamsAndMetricsTree private readonly experiments: Experiments private readonly resourceLocator: ResourceLocator - private pathRoots: Record = {} - private view: TreeView + private view: TreeView constructor(experiments: Experiments, resourceLocator: ResourceLocator) { this.resourceLocator = resourceLocator @@ -35,11 +41,14 @@ export class ExperimentsParamsAndMetricsTree this.onDidChangeTreeData = experiments.paramsOrMetricsChanged.event this.view = this.dispose.track( - window.createTreeView('dvc.views.experimentsParamsAndMetricsTree', { - canSelectMany: true, - showCollapseAll: true, - treeDataProvider: this - }) + window.createTreeView( + 'dvc.views.experimentsParamsAndMetricsTree', + { + canSelectMany: true, + showCollapseAll: true, + treeDataProvider: this + } + ) ) this.experiments = experiments @@ -48,7 +57,7 @@ export class ExperimentsParamsAndMetricsTree commands.registerCommand( 'dvc.views.experimentsParamsAndMetricsTree.toggleStatus', resource => { - const [dvcRoot, path] = this.getDetails(resource) + const { dvcRoot, path } = resource return this.experiments.toggleParamOrMetricStatus(dvcRoot, path) } ) @@ -57,42 +66,36 @@ export class ExperimentsParamsAndMetricsTree this.updateDescriptionOnChange() } - public getTreeItem(element: string): TreeItem { - const resourceUri = Uri.file(element) - - const [dvcRoot, path] = this.getDetails(element) - - if (!path) { + public getTreeItem(element: string | ParamsAndMetricsItem): TreeItem { + if (this.isRoot(element)) { + const resourceUri = Uri.file(element) return new TreeItem(resourceUri, TreeItemCollapsibleState.Collapsed) } - const paramOrMetric = this.experiments.getParamOrMetric(dvcRoot, path) - const hasChildren = !!paramOrMetric?.hasChildren - const descendantStatuses = paramOrMetric?.descendantStatuses + const { dvcRoot, path, collapsibleState, description, iconPath } = element const treeItem = new TreeItem( - resourceUri, - hasChildren - ? TreeItemCollapsibleState.Collapsed - : TreeItemCollapsibleState.None + Uri.file(join(dvcRoot, path)), + collapsibleState ) treeItem.command = { - arguments: [element], + arguments: [{ dvcRoot, path }], command: 'dvc.views.experimentsParamsAndMetricsTree.toggleStatus', title: 'toggle' } - treeItem.iconPath = this.getIconPath(paramOrMetric?.status) - - if (hasChildren && descendantStatuses) { - treeItem.description = this.getDescription(descendantStatuses, '/') + treeItem.iconPath = iconPath + if (description) { + treeItem.description = description } return treeItem } - public getChildren(element?: string): Promise { + public getChildren( + element?: string | ParamsAndMetricsItem + ): Promise { if (element) { return Promise.resolve(this.getParamsOrMetrics(element)) } @@ -117,9 +120,6 @@ export class ExperimentsParamsAndMetricsTree private async getRootElements() { await this.experiments.isReady() const dvcRoots = this.experiments.getDvcRoots() - dvcRoots.forEach(dvcRoot => { - this.pathRoots[dvcRoot] = dvcRoot - }) if (dvcRoots.length === 1) { const [onlyRepo] = dvcRoots @@ -129,38 +129,36 @@ export class ExperimentsParamsAndMetricsTree return dvcRoots.sort((a, b) => a.localeCompare(b)) } - private getParamsOrMetrics(element: string): string[] { + private getParamsOrMetrics( + element: string | ParamsAndMetricsItem + ): ParamsAndMetricsItem[] { if (!element) { return [] } const [dvcRoot, path] = this.getDetails(element) - const paramsOrMetrics = this.experiments.getChildParamsOrMetrics( - dvcRoot, - path - ) - return ( - paramsOrMetrics?.map(paramOrMetric => - this.processParamOrMetric(dvcRoot, paramOrMetric) - ) || [] - ) - } + return this.experiments + .getChildParamsOrMetrics(dvcRoot, path) + .map(paramOrMetric => { + const { descendantStatuses, hasChildren, path, status } = paramOrMetric - private processParamOrMetric(dvcRoot: string, paramOrMetric: ParamOrMetric) { - const absPath = join(dvcRoot, paramOrMetric.path) - this.pathRoots[absPath] = dvcRoot - return absPath - } + const description = this.getDescription(descendantStatuses, '/') + const iconPath = this.getIconPath(status) + const collapsibleState = hasChildren + ? TreeItemCollapsibleState.Collapsed + : TreeItemCollapsibleState.None - private getDetails(element: string) { - const dvcRoot = this.getRoot(element) - const path = relative(dvcRoot, element) - return [dvcRoot, path] + return { collapsibleState, description, dvcRoot, iconPath, path } + }) } - private getRoot(element: string) { - return this.pathRoots[element] + private getDetails(element: string | ParamsAndMetricsItem) { + if (this.isRoot(element)) { + return [element, ''] + } + const { dvcRoot, path } = element + return [dvcRoot, path] } private getIconPath(status?: Status) { @@ -183,4 +181,8 @@ export class ExperimentsParamsAndMetricsTree ).length }${separator}${statuses.length}` } + + private isRoot(element: string | ParamsAndMetricsItem): element is string { + return typeof element === 'string' + } } diff --git a/extension/src/experiments/repository.ts b/extension/src/experiments/repository.ts index e758119187..2d4f8ed798 100644 --- a/extension/src/experiments/repository.ts +++ b/extension/src/experiments/repository.ts @@ -80,10 +80,6 @@ export class ExperimentsRepository { return this.processManager.run('refresh') } - public getParamOrMetric(path: string) { - return this.paramsAndMetrics.getParamOrMetric(path) - } - public getChildParamsOrMetrics(path: string) { return this.paramsAndMetrics.getChildren(path) } diff --git a/extension/src/resourceLocator.ts b/extension/src/resourceLocator.ts index 855fd54fca..e924bcb3aa 100644 --- a/extension/src/resourceLocator.ts +++ b/extension/src/resourceLocator.ts @@ -1,7 +1,7 @@ import { Disposable } from '@hediet/std/disposable' import { Uri } from 'vscode' -type Resource = { dark: Uri; light: Uri } +export type Resource = { dark: Uri; light: Uri } export class ResourceLocator { public dispose = Disposable.fn() diff --git a/extension/src/test/suite/experiments/paramsAndMetrics/tree.test.ts b/extension/src/test/suite/experiments/paramsAndMetrics/tree.test.ts index bc54ddd743..2da4bb5dbe 100644 --- a/extension/src/test/suite/experiments/paramsAndMetrics/tree.test.ts +++ b/extension/src/test/suite/experiments/paramsAndMetrics/tree.test.ts @@ -1,11 +1,10 @@ -import { join, relative, resolve } from 'path' +import { join, resolve } from 'path' import { afterEach, beforeEach, describe, it, suite } from 'mocha' import { expect } from 'chai' import { stub, restore } from 'sinon' import { window, commands, Uri } from 'vscode' import { Disposable } from '../../../../extension' import complexExperimentsOutput from '../../../../experiments/webview/complex-output-example.json' -import { ExperimentsParamsAndMetricsTree } from '../../../../experiments/paramsAndMetrics/tree' import { Experiments } from '../../../../experiments' import { ExperimentsRepository } from '../../../../experiments/repository' import { Status } from '../../../../experiments/paramsAndMetrics/model' @@ -53,14 +52,7 @@ suite('Extension Test Suite', () => { describe('experimentsParamsAndMetricsTree', () => { it('should be able to toggle whether an experiments param or metric is selected with dvc.views.experimentsParamsAndMetricsTree.toggleStatus', async () => { - const relPath = join('params', paramsFile, 'learning_rate') - const absPath = join(dvcDemoPath, relPath) - - stub( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (ExperimentsParamsAndMetricsTree as any).prototype, - 'getDetails' - ).returns([dvcDemoPath, relPath]) + const path = join('params', paramsFile, 'learning_rate') const config = disposable.track(new Config()) const cliReader = disposable.track(new CliReader(config)) @@ -88,33 +80,30 @@ suite('Extension Test Suite', () => { experimentsRepository ) - const isUnselected = await commands.executeCommand(toggleCommand, absPath) + const isUnselected = await commands.executeCommand(toggleCommand, { + dvcRoot: dvcDemoPath, + path + }) expect(isUnselected).to.equal(Status.unselected) - const isSelected = await commands.executeCommand(toggleCommand, absPath) + const isSelected = await commands.executeCommand(toggleCommand, { + dvcRoot: dvcDemoPath, + path + }) expect(isSelected).to.equal(Status.selected) - const isUnselectedAgain = await commands.executeCommand( - toggleCommand, - absPath - ) + const isUnselectedAgain = await commands.executeCommand(toggleCommand, { + dvcRoot: dvcDemoPath, + path + }) expect(isUnselectedAgain).to.equal(Status.unselected) }) it('should be able to toggle a parent and change the selected status of all of the children with dvc.views.experimentsParamsAndMetricsTree.toggleStatus', async () => { - const toggleCommand = - 'dvc.views.experimentsParamsAndMetricsTree.toggleStatus' - const relPath = join('params', paramsFile) - const absPath = join(dvcDemoPath, relPath) - - stub( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (ExperimentsParamsAndMetricsTree as any).prototype, - 'getDetails' - ).callsFake((path: string) => [dvcDemoPath, relative(dvcDemoPath, path)]) + const path = join('params', paramsFile) const config = disposable.track(new Config()) const cliReader = disposable.track(new CliReader(config)) @@ -143,71 +132,52 @@ suite('Extension Test Suite', () => { ) const selectedChildren = - experimentsRepository.getChildParamsOrMetrics(relPath) || [] + experimentsRepository.getChildParamsOrMetrics(path) || [] expect(selectedChildren).to.have.lengthOf.greaterThan(1) const selectedGrandChildren = - experimentsRepository.getChildParamsOrMetrics( - join(relPath, 'process') - ) || [] + experimentsRepository.getChildParamsOrMetrics(join(path, 'process')) || + [] expect(selectedGrandChildren).to.have.lengthOf.greaterThan(1) - const allChildren = [...selectedChildren, ...selectedGrandChildren] + const allSelectedChildren = [ + ...selectedChildren, + ...selectedGrandChildren + ] - allChildren.map(paramOrMetric => - expect( - experimentsRepository.getParamOrMetric(paramOrMetric.path)?.status - ).to.equal(Status.selected) + allSelectedChildren.map(paramOrMetric => + expect(paramOrMetric.status).to.equal(Status.selected) ) - expect( - experimentsRepository.getParamOrMetric(relPath)?.descendantStatuses - ).to.deep.equal([ - Status.selected, - Status.selected, - Status.selected, - Status.selected, - Status.selected, - Status.selected, - Status.selected, - Status.selected - ]) - - const isUnselected = await commands.executeCommand(toggleCommand, absPath) + const isUnselected = await commands.executeCommand(toggleCommand, { + dvcRoot: dvcDemoPath, + path + }) expect(isUnselected).to.equal(Status.unselected) - allChildren.map(paramOrMetric => - expect( - experimentsRepository.getParamOrMetric(paramOrMetric.path)?.status - ).to.equal(isUnselected) - ) + const unselectedChildren = + experimentsRepository.getChildParamsOrMetrics(path) || [] + expect(selectedChildren).to.have.lengthOf.greaterThan(1) + + const unselectedGrandChildren = + experimentsRepository.getChildParamsOrMetrics(join(path, 'process')) || + [] - expect( - experimentsRepository.getParamOrMetric(relPath)?.descendantStatuses - ).to.deep.equal([ - Status.unselected, - Status.unselected, - Status.unselected, - Status.unselected, - Status.unselected, - Status.unselected, - Status.unselected, - Status.unselected - ]) + const allUnselectedChildren = [ + ...unselectedChildren, + ...unselectedGrandChildren + ] + + allUnselectedChildren.map(paramOrMetric => + expect(paramOrMetric.status).to.equal(Status.unselected) + ) }) }) it("should be able to select a child and set all of the ancestors' statuses to indeterminate with dvc.views.experimentsParamsAndMetricsTree.toggleStatus", async () => { const grandParentPath = join('params', paramsFile) const parentPath = join(grandParentPath, 'process') - const absPath = join(dvcDemoPath, grandParentPath) - - stub( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (ExperimentsParamsAndMetricsTree as any).prototype, - 'getDetails' - ).callsFake((path: string) => [dvcDemoPath, relative(dvcDemoPath, path)]) const config = disposable.track(new Config()) const cliReader = disposable.track(new CliReader(config)) @@ -231,69 +201,60 @@ suite('Extension Test Suite', () => { experimentsRepository ) - const selectedChildren = + await commands.executeCommand(toggleCommand, { + dvcRoot: dvcDemoPath, + path: grandParentPath + }) + + const unselectedChildren = experimentsRepository.getChildParamsOrMetrics(grandParentPath) || [] - expect(selectedChildren).to.have.lengthOf.greaterThan(1) + expect(unselectedChildren).to.have.lengthOf.greaterThan(1) - const selectedGrandChildren = + const unselectedGrandChildren = experimentsRepository.getChildParamsOrMetrics(parentPath) || [] - expect(selectedGrandChildren).to.have.lengthOf.greaterThan(1) + expect(unselectedGrandChildren).to.have.lengthOf.greaterThan(1) - const allParamsAndMetrics = [ - ...selectedChildren, - { path: grandParentPath }, - ...selectedGrandChildren - ] + const allUnselected = [...unselectedChildren, ...unselectedGrandChildren] - await commands.executeCommand(toggleCommand, absPath) - - allParamsAndMetrics.map(paramOrMetric => - expect( - experimentsRepository.getParamOrMetric(paramOrMetric.path)?.status - ).to.equal(Status.unselected) + allUnselected.map(paramOrMetric => + expect(paramOrMetric?.status).to.equal(Status.unselected) ) - const [firstGrandChild] = selectedGrandChildren + const [firstGrandChild] = unselectedGrandChildren - const isSelected = await commands.executeCommand( - toggleCommand, - join(dvcDemoPath, firstGrandChild.path) - ) + const isSelected = await commands.executeCommand(toggleCommand, { + dvcRoot: dvcDemoPath, + path: firstGrandChild.path + }) expect(isSelected).to.equal(Status.selected) - const parentParam = experimentsRepository.getParamOrMetric(parentPath) - expect(parentParam?.status).to.equal(Status.indeterminate) - expect(parentParam?.descendantStatuses).to.deep.equal([ - Status.selected, - Status.unselected - ]) - - const grandParentParam = - experimentsRepository.getParamOrMetric(grandParentPath) - expect(grandParentParam?.status).to.equal(Status.indeterminate) - expect(grandParentParam?.descendantStatuses).to.deep.equal([ - Status.unselected, - Status.unselected, - Status.unselected, - Status.unselected, - Status.unselected, - Status.indeterminate, - Status.selected, - Status.unselected - ]) + const indeterminateChildren = + experimentsRepository.getChildParamsOrMetrics(parentPath) || [] + + expect( + indeterminateChildren.map(paramOrMetric => paramOrMetric.status) + ).to.deep.equal([Status.selected, Status.unselected]) + + const unselectedOrIndeterminateParams = + experimentsRepository.getChildParamsOrMetrics(grandParentPath) || [] + + expect( + unselectedOrIndeterminateParams.find( + paramOrMetric => paramOrMetric.path === parentPath + )?.status + ).to.equal(Status.indeterminate) + + unselectedOrIndeterminateParams + .filter(paramOrMetric => paramOrMetric.path !== parentPath) + .map(paramOrMetric => + expect(paramOrMetric.status).to.equal(Status.unselected) + ) }) it("should be able to unselect the last remaining selected child and set it's ancestors to unselected with dvc.views.experimentsParamsAndMetricsTree.toggleStatus", async () => { const grandParentPath = join('params', paramsFile) const parentPath = join(grandParentPath, 'process') - const absPath = join(dvcDemoPath, grandParentPath) - - stub( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - (ExperimentsParamsAndMetricsTree as any).prototype, - 'getDetails' - ).callsFake((path: string) => [dvcDemoPath, relative(dvcDemoPath, path)]) const config = disposable.track(new Config()) const cliReader = disposable.track(new CliReader(config)) @@ -321,49 +282,54 @@ suite('Extension Test Suite', () => { experimentsRepository.getChildParamsOrMetrics(parentPath) || [] expect(selectedGrandChildren).to.have.lengthOf.greaterThan(1) - await commands.executeCommand(toggleCommand, absPath) + await commands.executeCommand(toggleCommand, { + dvcRoot: dvcDemoPath, + path: grandParentPath + }) expect(selectedGrandChildren).to.have.lengthOf(2) - const [firstGrandChild, secondGrandChild] = selectedGrandChildren + const [firstGrandChild] = selectedGrandChildren - const isSelected = await commands.executeCommand( - toggleCommand, - join(dvcDemoPath, firstGrandChild.path) + selectedGrandChildren.map(paramOrMetric => + expect(paramOrMetric.status).to.equal(Status.selected) ) - expect(isSelected).to.equal(Status.selected) + await commands.executeCommand(toggleCommand, { + dvcRoot: dvcDemoPath, + path: firstGrandChild.path + }) + + const indeterminateGrandChildren = + experimentsRepository.getChildParamsOrMetrics(parentPath) || [] + expect(selectedGrandChildren).to.have.lengthOf.greaterThan(1) expect( - experimentsRepository.getParamOrMetric(secondGrandChild.path)?.status - ).to.equal(Status.unselected) + indeterminateGrandChildren.map(paramOrMetric => paramOrMetric.status) + ).to.deep.equal([Status.selected, Status.unselected]) const lastSelectedIsUnselected = await commands.executeCommand( toggleCommand, - join(dvcDemoPath, firstGrandChild.path) + { + dvcRoot: dvcDemoPath, + path: firstGrandChild.path + } ) expect(lastSelectedIsUnselected).to.equal(Status.unselected) - const parentParam = experimentsRepository.getParamOrMetric(parentPath) - expect(parentParam?.status).to.equal(lastSelectedIsUnselected) - expect(parentParam?.descendantStatuses).to.deep.equal([ - Status.unselected, - Status.unselected - ]) - - const grandParentParam = - experimentsRepository.getParamOrMetric(grandParentPath) - expect(grandParentParam?.status).to.equal(lastSelectedIsUnselected) - expect(grandParentParam?.descendantStatuses).to.deep.equal([ - Status.unselected, - Status.unselected, - Status.unselected, - Status.unselected, - Status.unselected, - Status.unselected, - Status.unselected, - Status.unselected - ]) + const unselectedChildren = + experimentsRepository.getChildParamsOrMetrics(parentPath) || [] + + unselectedChildren.map(paramOrMetric => + expect(paramOrMetric.status).to.equal(Status.unselected) + ) + + const unselectedParents = + experimentsRepository.getChildParamsOrMetrics(grandParentPath) || [] + + unselectedParents.map(paramOrMetric => + expect(paramOrMetric.status).to.equal(Status.unselected) + ) }) })