From fcba9cecf9c97d7d7105066a43fc2e6f0d8f2c04 Mon Sep 17 00:00:00 2001 From: julieg18 Date: Mon, 26 Sep 2022 11:42:55 -0500 Subject: [PATCH 1/2] Add tests for table depth config --- .../src/experiments/columns/collect/util.ts | 11 +- extension/src/experiments/columns/consts.ts | 1 - .../src/experiments/columns/model.test.ts | 33 ++- .../src/test/fixtures/expShow/deeplyNested.ts | 261 ++++++++++++++++++ 4 files changed, 299 insertions(+), 7 deletions(-) delete mode 100644 extension/src/experiments/columns/consts.ts diff --git a/extension/src/experiments/columns/collect/util.ts b/extension/src/experiments/columns/collect/util.ts index 4f1e2c1fa6..00e567a62e 100644 --- a/extension/src/experiments/columns/collect/util.ts +++ b/extension/src/experiments/columns/collect/util.ts @@ -1,11 +1,14 @@ import { Column, ColumnType } from '../../webview/contract' import { Value } from '../../../cli/dvc/reader' import { ConfigKey, getConfigValue } from '../../../vscode/config' -import { INITIAL_TABLE_HEAD_MAX_LAYERS } from '../consts' export type ColumnAccumulator = Record -export const limitAncestorDepth = (ancestors: string[], sep: string) => { +export const limitAncestorDepth = ( + ancestors: string[], + sep: string, + limit = 5 +) => { const [path, ...rest] = ancestors /* The depth is only limited for the middle of the path array. @@ -14,9 +17,7 @@ export const limitAncestorDepth = (ancestors: string[], sep: string) => { from what we want the final layer count to be. */ const collectedLimit = - Number(getConfigValue(ConfigKey.EXP_TABLE_HEAD_MAX_LAYERS)) || - INITIAL_TABLE_HEAD_MAX_LAYERS - + Number(getConfigValue(ConfigKey.EXP_TABLE_HEAD_MAX_LAYERS)) || limit const convertedLimit = (collectedLimit >= 3 ? collectedLimit : 3) - 3 if (rest.length <= convertedLimit) { return ancestors diff --git a/extension/src/experiments/columns/consts.ts b/extension/src/experiments/columns/consts.ts deleted file mode 100644 index 050b5e5e53..0000000000 --- a/extension/src/experiments/columns/consts.ts +++ /dev/null @@ -1 +0,0 @@ -export const INITIAL_TABLE_HEAD_MAX_LAYERS = 5 diff --git a/extension/src/experiments/columns/model.test.ts b/extension/src/experiments/columns/model.test.ts index e51b99f076..d7eb1222e7 100644 --- a/extension/src/experiments/columns/model.test.ts +++ b/extension/src/experiments/columns/model.test.ts @@ -9,30 +9,61 @@ import outputFixture from '../../test/fixtures/expShow/output' import columnsFixture from '../../test/fixtures/expShow/columns' import { deeplyNestedOutput, - columns as deeplyNestedColumns + columns as deeplyNestedColumns, + columnsWithDepthOf10 as deeplyNestedColumnsWithDepthOf10, + columnsWithDepthOf3 as deeplyNestedColumnsWithDepthOf3 } from '../../test/fixtures/expShow/deeplyNested' import { dataTypesOutput, columns as dataTypesColumns } from '../../test/fixtures/expShow/dataTypes' +import { getConfigValue } from '../../vscode/config' jest.mock('../../vscode/config') +const mockedGetConfigValue = jest.mocked(getConfigValue) + describe('ColumnsModel', () => { const exampleDvcRoot = 'test' it('should return the expected columns when given the default output fixture', async () => { const model = new ColumnsModel('', buildMockMemento()) await model.transformAndSet(outputFixture) + expect(mockedGetConfigValue).toHaveBeenCalled() expect(model.getSelected()).toStrictEqual(columnsFixture) }) it('should return the expected columns when given the deeply nested output fixture', async () => { const model = new ColumnsModel('', buildMockMemento()) await model.transformAndSet(deeplyNestedOutput) + expect(mockedGetConfigValue).toHaveBeenCalled() expect(model.getSelected()).toStrictEqual(deeplyNestedColumns) }) + it('should return the expected columns when the max depth config is set to 10', async () => { + mockedGetConfigValue.mockImplementation(() => 10) + const model = new ColumnsModel('', buildMockMemento()) + await model.transformAndSet(deeplyNestedOutput) + expect(mockedGetConfigValue).toHaveBeenCalled() + expect(model.getSelected()).toStrictEqual(deeplyNestedColumnsWithDepthOf10) + }) + + it('should return the expected columns when the max depth config is set to 3', async () => { + mockedGetConfigValue.mockImplementation(() => 3) + const model = new ColumnsModel('', buildMockMemento()) + await model.transformAndSet(deeplyNestedOutput) + expect(mockedGetConfigValue).toHaveBeenCalled() + expect(model.getSelected()).toStrictEqual(deeplyNestedColumnsWithDepthOf3) + }) + + it('should return the expected columns when the max depth config is set to -1', async () => { + mockedGetConfigValue.mockImplementation(() => -1) + const model = new ColumnsModel('', buildMockMemento()) + await model.transformAndSet(deeplyNestedOutput) + expect(mockedGetConfigValue).toHaveBeenCalled() + expect(model.getSelected()).toStrictEqual(deeplyNestedColumnsWithDepthOf3) + }) + it('should return the expected columns when given the data types output fixture', async () => { const model = new ColumnsModel('', buildMockMemento()) await model.transformAndSet(dataTypesOutput) diff --git a/extension/src/test/fixtures/expShow/deeplyNested.ts b/extension/src/test/fixtures/expShow/deeplyNested.ts index 002a826f5f..4ca40aad17 100644 --- a/extension/src/test/fixtures/expShow/deeplyNested.ts +++ b/extension/src/test/fixtures/expShow/deeplyNested.ts @@ -216,6 +216,267 @@ export const columns: Column[] = [ } ] +export const columnsWithDepthOf10: Column[] = [ + timestampColumn, + { + hasChildren: true, + label: 'params.yaml', + parentPath: 'params', + path: 'params:params.yaml', + type: ColumnType.PARAMS + }, + { + hasChildren: true, + label: 'nested1', + parentPath: 'params:params.yaml', + path: 'params:params.yaml:nested1', + type: ColumnType.PARAMS + }, + { + hasChildren: false, + label: 'doubled', + maxStringLength: 15, + parentPath: 'params:params.yaml:nested1', + path: 'params:params.yaml:nested1.doubled', + pathArray: ['params', 'params.yaml', 'nested1', 'doubled'], + type: ColumnType.PARAMS, + types: ['string'] + }, + { + hasChildren: true, + label: 'nested2', + parentPath: 'params:params.yaml:nested1', + path: 'params:params.yaml:nested1.nested2', + type: ColumnType.PARAMS + }, + + { + hasChildren: true, + label: 'nested3', + parentPath: 'params:params.yaml:nested1.nested2', + path: 'params:params.yaml:nested1.nested2.nested3', + type: ColumnType.PARAMS + }, + { + hasChildren: true, + label: 'nested4', + parentPath: 'params:params.yaml:nested1.nested2.nested3', + path: 'params:params.yaml:nested1.nested2.nested3.nested4', + type: ColumnType.PARAMS + }, + { + hasChildren: true, + label: 'nested5', + parentPath: 'params:params.yaml:nested1.nested2.nested3.nested4', + path: 'params:params.yaml:nested1.nested2.nested3.nested4.nested5', + type: ColumnType.PARAMS + }, + { + hasChildren: true, + label: 'nested6', + parentPath: 'params:params.yaml:nested1.nested2.nested3.nested4.nested5', + path: 'params:params.yaml:nested1.nested2.nested3.nested4.nested5.nested6', + type: ColumnType.PARAMS + }, + { + hasChildren: false, + label: 'nested7', + maxStringLength: 6, + parentPath: + 'params:params.yaml:nested1.nested2.nested3.nested4.nested5.nested6', + path: 'params:params.yaml:nested1.nested2.nested3.nested4.nested5.nested6.nested7', + pathArray: [ + 'params', + 'params.yaml', + 'nested1', + 'nested2', + 'nested3', + 'nested4', + 'nested5', + 'nested6', + 'nested7' + ], + type: ColumnType.PARAMS, + types: ['string'] + }, + { + hasChildren: true, + label: 'nested5b', + parentPath: 'params:params.yaml:nested1.nested2.nested3.nested4', + path: 'params:params.yaml:nested1.nested2.nested3.nested4.nested5b', + type: ColumnType.PARAMS + }, + { + hasChildren: false, + label: 'nested6', + maxStringLength: 23, + parentPath: 'params:params.yaml:nested1.nested2.nested3.nested4.nested5b', + path: 'params:params.yaml:nested1.nested2.nested3.nested4.nested5b.nested6', + pathArray: [ + 'params', + 'params.yaml', + 'nested1', + 'nested2', + 'nested3', + 'nested4', + 'nested5b', + 'nested6' + ], + type: ColumnType.PARAMS, + types: ['string'] + }, + { + hasChildren: false, + label: 'doubled', + maxStringLength: 16, + parentPath: 'params:params.yaml:nested1.nested2.nested3.nested4.nested5b', + path: 'params:params.yaml:nested1.nested2.nested3.nested4.nested5b.doubled', + pathArray: [ + 'params', + 'params.yaml', + 'nested1', + 'nested2', + 'nested3', + 'nested4', + 'nested5b', + 'doubled' + ], + type: ColumnType.PARAMS, + types: ['string'] + }, + { + hasChildren: false, + label: 'outlier', + maxStringLength: 1, + parentPath: 'params:params.yaml', + path: 'params:params.yaml:outlier', + pathArray: ['params', 'params.yaml', 'outlier'], + type: ColumnType.PARAMS, + types: ['number'], + maxNumber: 1, + minNumber: 1 + } +] + +export const columnsWithDepthOf3: Column[] = [ + timestampColumn, + { + hasChildren: true, + label: 'params.yaml', + parentPath: 'params', + path: 'params:params.yaml', + type: ColumnType.PARAMS + }, + { + hasChildren: true, + label: 'nested1', + parentPath: 'params:params.yaml', + path: 'params:params.yaml:nested1', + type: ColumnType.PARAMS + }, + + { + hasChildren: false, + label: 'doubled', + maxStringLength: 15, + parentPath: 'params:params.yaml:nested1', + path: 'params:params.yaml:nested1.doubled', + pathArray: ['params', 'params.yaml', 'nested1', 'doubled'], + type: ColumnType.PARAMS, + types: ['string'] + }, + { + hasChildren: true, + label: 'nested1.nested2.nested3.nested4.nested5.nested6', + parentPath: 'params:params.yaml', + path: 'params:params.yaml:nested1%2Enested2%2Enested3%2Enested4%2Enested5%2Enested6', + type: ColumnType.PARAMS + }, + { + hasChildren: false, + label: 'nested7', + maxStringLength: 6, + parentPath: + 'params:params.yaml:nested1%2Enested2%2Enested3%2Enested4%2Enested5%2Enested6', + path: 'params:params.yaml:nested1%2Enested2%2Enested3%2Enested4%2Enested5%2Enested6.nested7', + pathArray: [ + 'params', + 'params.yaml', + 'nested1', + 'nested2', + 'nested3', + 'nested4', + 'nested5', + 'nested6', + 'nested7' + ], + type: ColumnType.PARAMS, + types: ['string'] + }, + + { + hasChildren: true, + label: 'nested1.nested2.nested3.nested4.nested5b', + parentPath: 'params:params.yaml', + path: 'params:params.yaml:nested1%2Enested2%2Enested3%2Enested4%2Enested5b', + type: ColumnType.PARAMS + }, + { + hasChildren: false, + label: 'nested6', + maxStringLength: 23, + parentPath: + 'params:params.yaml:nested1%2Enested2%2Enested3%2Enested4%2Enested5b', + path: 'params:params.yaml:nested1%2Enested2%2Enested3%2Enested4%2Enested5b.nested6', + pathArray: [ + 'params', + 'params.yaml', + 'nested1', + 'nested2', + 'nested3', + 'nested4', + 'nested5b', + 'nested6' + ], + type: ColumnType.PARAMS, + types: ['string'] + }, + + { + hasChildren: false, + label: 'doubled', + maxStringLength: 16, + parentPath: + 'params:params.yaml:nested1%2Enested2%2Enested3%2Enested4%2Enested5b', + path: 'params:params.yaml:nested1%2Enested2%2Enested3%2Enested4%2Enested5b.doubled', + pathArray: [ + 'params', + 'params.yaml', + 'nested1', + 'nested2', + 'nested3', + 'nested4', + 'nested5b', + 'doubled' + ], + type: ColumnType.PARAMS, + types: ['string'] + }, + + { + hasChildren: false, + label: 'outlier', + maxStringLength: 1, + parentPath: 'params:params.yaml', + path: 'params:params.yaml:outlier', + pathArray: ['params', 'params.yaml', 'outlier'], + type: ColumnType.PARAMS, + types: ['number'], + maxNumber: 1, + minNumber: 1 + } +] + export const rows = [ { id: 'workspace', From 252020b530f4b3777ee19fcb5bbd8e55e98d9f2a Mon Sep 17 00:00:00 2001 From: julieg18 Date: Mon, 26 Sep 2022 17:22:35 -0500 Subject: [PATCH 2/2] Use default value in getConfiguration --- extension/src/experiments/columns/collect/index.test.ts | 4 ++++ extension/src/experiments/columns/collect/util.ts | 5 +++-- extension/src/experiments/columns/model.test.ts | 1 + extension/src/vscode/config.ts | 7 +++++-- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/extension/src/experiments/columns/collect/index.test.ts b/extension/src/experiments/columns/collect/index.test.ts index d653539902..5ec0eea358 100644 --- a/extension/src/experiments/columns/collect/index.test.ts +++ b/extension/src/experiments/columns/collect/index.test.ts @@ -8,9 +8,13 @@ import columnsFixture from '../../../test/fixtures/expShow/columns' import workspaceChangesFixture from '../../../test/fixtures/expShow/workspaceChanges' import uncommittedDepsFixture from '../../../test/fixtures/expShow/uncommittedDeps' import { ExperimentsOutput } from '../../../cli/dvc/reader' +import { getConfigValue } from '../../../vscode/config' jest.mock('../../../vscode/config') +const mockedGetConfigValue = jest.mocked(getConfigValue) +mockedGetConfigValue.mockImplementation(() => 5) + describe('collectColumns', () => { it('should return a value equal to the columns fixture when given the output fixture', () => { const columns = collectColumns(outputFixture) diff --git a/extension/src/experiments/columns/collect/util.ts b/extension/src/experiments/columns/collect/util.ts index 00e567a62e..684eb81bc3 100644 --- a/extension/src/experiments/columns/collect/util.ts +++ b/extension/src/experiments/columns/collect/util.ts @@ -16,8 +16,9 @@ export const limitAncestorDepth = ( concatenated layer itself counts as one; because of this, we must subtract 3 from what we want the final layer count to be. */ - const collectedLimit = - Number(getConfigValue(ConfigKey.EXP_TABLE_HEAD_MAX_LAYERS)) || limit + const collectedLimit = Number( + getConfigValue(ConfigKey.EXP_TABLE_HEAD_MAX_LAYERS, limit) + ) const convertedLimit = (collectedLimit >= 3 ? collectedLimit : 3) - 3 if (rest.length <= convertedLimit) { return ancestors diff --git a/extension/src/experiments/columns/model.test.ts b/extension/src/experiments/columns/model.test.ts index d7eb1222e7..e08489891a 100644 --- a/extension/src/experiments/columns/model.test.ts +++ b/extension/src/experiments/columns/model.test.ts @@ -22,6 +22,7 @@ import { getConfigValue } from '../../vscode/config' jest.mock('../../vscode/config') const mockedGetConfigValue = jest.mocked(getConfigValue) +mockedGetConfigValue.mockImplementation(() => 5) describe('ColumnsModel', () => { const exampleDvcRoot = 'test' diff --git a/extension/src/vscode/config.ts b/extension/src/vscode/config.ts index eaf32ca09e..ad3e91f9f9 100644 --- a/extension/src/vscode/config.ts +++ b/extension/src/vscode/config.ts @@ -10,8 +10,11 @@ export enum ConfigKey { PYTHON_PATH = 'dvc.pythonPath' } -export const getConfigValue = (key: ConfigKey): T => - workspace.getConfiguration().get(key, '') as unknown as T +export const getConfigValue = ( + key: ConfigKey, + defaultValue?: D | T +): T => + workspace.getConfiguration().get(key, defaultValue ?? '') as unknown as T export const setConfigValue = (key: ConfigKey, value: unknown) => workspace.getConfiguration().update(key, value)