Fix non-standard data type tooltips and add tests

extension/src/cli/reader.ts

@@ -49,14 +49,8 @@ export type StatusesOrAlwaysChanged = StageOrFileStatuses | 'always changed'
 
 export type StatusOutput = Record<string, StatusesOrAlwaysChanged[]>
 
-export type Value =
-  | string
-  | number
-  | boolean
-  | null
-  | number[]
-  | string[]
-  | boolean[]
+type SingleValue = string | number | boolean | null
+export type Value = SingleValue | SingleValue[]
 
 export interface ValueTreeOrError {
   data?: ValueTree

extension/src/experiments/columns/model.test.ts

@@ -10,21 +10,32 @@ import {
   deeplyNestedOutput,
   columns as deeplyNestedColumns
 } from '../../test/fixtures/expShow/deeplyNested'
+import {
+  dataTypesOutput,
+  columns as dataTypesColumns
+} from '../../test/fixtures/expShow/dataTypes'
 
 describe('ColumnsModel', () => {
   const exampleDvcRoot = 'test'
 
-  it('should return columns that equal the columns fixture when given the output fixture', async () => {
+  it('should return the expected columns when given the default output fixture', async () => {
     const model = new ColumnsModel('', buildMockMemento())
     await model.transformAndSet(outputFixture)
     expect(model.getSelected()).toStrictEqual(columnsFixture)
   })
 
-  it('should return data that equal the deeply nested output fixture', async () => {
+  it('should return the expected columns when given the deeply nested output fixture', async () => {
     const model = new ColumnsModel('', buildMockMemento())
     await model.transformAndSet(deeplyNestedOutput)
     expect(model.getSelected()).toStrictEqual(deeplyNestedColumns)
   })
+
+  it('should return the expected columns when given the data types output fixture', async () => {
+    const model = new ColumnsModel('', buildMockMemento())
+    await model.transformAndSet(dataTypesOutput)
+    expect(model.getSelected()).toStrictEqual(dataTypesColumns)
+  })
+
   describe('persistence', () => {
     const paramsDotYamlPath = buildMetricOrParamPath(
       ColumnType.PARAMS,

extension/src/experiments/model/index.test.ts

@@ -12,6 +12,10 @@ import { buildMockMemento } from '../../test/util'
 import { buildMetricOrParamPath } from '../columns/paths'
 import { Experiment, ColumnType } from '../webview/contract'
 import { definedAndNonEmpty } from '../../util/array'
+import {
+  dataTypesOutput,
+  rows as dataTypesRows
+} from '../../test/fixtures/expShow/dataTypes'
 
 jest.mock('vscode')
 
@@ -64,18 +68,24 @@ describe('ExperimentsModel', () => {
     return { data }
   }
 
-  it('should return rows that equal the rows fixture when given the output fixture', () => {
+  it('should return the expected rows when given the output fixture', () => {
     const model = new ExperimentsModel('', buildMockMemento())
     model.transformAndSet(outputFixture)
     expect(model.getRowData()).toStrictEqual(rowsFixture)
   })
 
-  it('should return data that equal the deeply nested output fixture', () => {
+  it('should return the expected rows when given the deeply nested output fixture', () => {
     const model = new ExperimentsModel('', buildMockMemento())
     model.transformAndSet(deeplyNestedOutput)
     expect(model.getRowData()).toStrictEqual(deeplyNestedRows)
   })
 
+  it('should return the expected rows when given the data types output fixture', () => {
+    const model = new ExperimentsModel('', buildMockMemento())
+    model.transformAndSet(dataTypesOutput)
+    expect(model.getRowData()).toStrictEqual(dataTypesRows)
+  })
+
   it('should continue to apply filters to new data if selection mode is set to use filters', () => {
     const testPath = buildMetricOrParamPath(
       ColumnType.PARAMS,

extension/src/test/fixtures/expShow/dataTypes.ts

@@ -0,0 +1,193 @@
+import { ExperimentsOutput } from '../../../cli/reader'
+import {
+  Column,
+  ColumnType,
+  Row,
+  TableData
+} from '../../../experiments/webview/contract'
+
+export const dataTypesOutput: ExperimentsOutput = {
+  workspace: {
+    baseline: {
+      data: {
+        timestamp: null,
+        params: {
+          'params.yaml': {
+            data: {
+              bool1: true,
+              bool2: false,
+              zero: 0,
+              negative: -123,
+              float: 1.9293040037155151,
+              string: 'string',
+              emptyString: '',
+              array: [true, false, 'string', 2]
+            }
+          }
+        },
+        queued: false,
+        running: false,
+        executor: null
+      }
+    }
+  },
+  '53c3851f46955fa3e2b8f6e1c52999acc8c9ea77': {
+    baseline: {
+      data: {
+        timestamp: '2020-11-21T19:58:22',
+        queued: false,
+        running: false,
+        executor: null,
+        name: 'main'
+      }
+    }
+  }
+}
+
+export const columns: Column[] = [
+  {
+    hasChildren: true,
+    label: 'params.yaml',
+    parentPath: ColumnType.PARAMS,
+    path: 'params:params.yaml',
+    type: ColumnType.PARAMS
+  },
+  {
+    hasChildren: false,
+    label: 'bool1',
+    maxStringLength: 4,
+    parentPath: 'params:params.yaml',
+    path: 'params:params.yaml:bool1',
+    pathArray: [ColumnType.PARAMS, 'params.yaml', 'bool1'],
+    type: ColumnType.PARAMS,
+    types: ['boolean']
+  },
+  {
+    hasChildren: false,
+    label: 'bool2',
+    maxStringLength: 5,
+    parentPath: 'params:params.yaml',
+    path: 'params:params.yaml:bool2',
+    pathArray: [ColumnType.PARAMS, 'params.yaml', 'bool2'],
+    type: ColumnType.PARAMS,
+    types: ['boolean']
+  },
+  {
+    hasChildren: false,
+    label: 'zero',
+    maxNumber: 0,
+    maxStringLength: 1,
+    minNumber: 0,
+    parentPath: 'params:params.yaml',
+    path: 'params:params.yaml:zero',
+    pathArray: [ColumnType.PARAMS, 'params.yaml', 'zero'],
+    type: ColumnType.PARAMS,
+    types: ['number']
+  },
+  {
+    hasChildren: false,
+    label: 'negative',
+    maxNumber: -123,
+    maxStringLength: 4,
+    minNumber: -123,
+    parentPath: 'params:params.yaml',
+    path: 'params:params.yaml:negative',
+    pathArray: [ColumnType.PARAMS, 'params.yaml', 'negative'],
+    type: ColumnType.PARAMS,
+    types: ['number']
+  },
+  {
+    hasChildren: false,
+    label: 'float',
+    maxNumber: 1.9293040037155151,
+    maxStringLength: 18,
+    minNumber: 1.9293040037155151,
+    parentPath: 'params:params.yaml',
+    path: 'params:params.yaml:float',
+    pathArray: [ColumnType.PARAMS, 'params.yaml', 'float'],
+    type: ColumnType.PARAMS,
+    types: ['number']
+  },
+  {
+    hasChildren: false,
+    label: 'string',
+    maxStringLength: 6,
+    parentPath: 'params:params.yaml',
+    path: 'params:params.yaml:string',
+    pathArray: [ColumnType.PARAMS, 'params.yaml', 'string'],
+    type: ColumnType.PARAMS,
+    types: ['string']
+  },
+  {
+    hasChildren: false,
+    label: 'emptyString',
+    maxStringLength: 0,
+    parentPath: 'params:params.yaml',
+    path: 'params:params.yaml:emptyString',
+    pathArray: [ColumnType.PARAMS, 'params.yaml', 'emptyString'],
+    type: ColumnType.PARAMS,
+    types: ['string']
+  },
+  {
+    hasChildren: false,
+    label: 'array',
+    maxStringLength: 19,
+    parentPath: 'params:params.yaml',
+    path: 'params:params.yaml:array',
+    pathArray: [ColumnType.PARAMS, 'params.yaml', 'array'],
+    type: ColumnType.PARAMS,
+    types: ['array']
+  }
+]
+export const rows: Row[] = [
+  {
+    displayColor: '#945dd6',
+    executor: null,
+    id: 'workspace',
+    label: 'workspace',
+    params: {
+      'params.yaml': {
+        array: [true, false, 'string', 2],
+        emptyString: '',
+        bool2: false,
+        float: 1.9293040037155151,
+        negative: -123,
+        string: 'string',
+        bool1: true,
+        zero: 0
+      }
+    },
+    queued: false,
+    running: false,
+    selected: true,
+    timestamp: null
+  },
+  {
+    displayColor: '#13adc7',
+    executor: null,
+    id: 'main',
+    label: 'main',
+    name: 'main',
+    queued: false,
+    running: false,
+    selected: true,
+    sha: '53c3851f46955fa3e2b8f6e1c52999acc8c9ea77',
+    timestamp: '2020-11-21T19:58:22'
+  }
+]
+
+export const dataTypesTableData: TableData = {
+  changes: [],
+  columnOrder: [],
+  columnWidths: {},
+  filteredCounts: { experiments: 0, checkpoints: 0 },
+  filters: [],
+  hasCheckpoints: false,
+  hasRunningExperiment: false,
+  sorts: [],
+  columns,
+  hasColumns: true,
+  rows
+}
+
+export default dataTypesTableData

webview/src/experiments/components/App.test.tsx

@@ -25,6 +25,7 @@ import {
   TableData
 } from 'dvc/src/experiments/webview/contract'
 import { buildMetricOrParamPath } from 'dvc/src/experiments/columns/paths'
+import { dataTypesTableData } from 'dvc/src/test/fixtures/expShow/dataTypes'
 import { App } from './App'
 import { useIsFullyContained } from './overflowHoverTooltip/useIsFullyContained'
 import styles from './table/styles.module.scss'
@@ -613,9 +614,6 @@ describe('App', () => {
     })
 
     it('should show and hide a tooltip on mouseEnter and mouseLeave of a cell', () => {
-      jest
-        .spyOn(window, 'requestAnimationFrame')
-        .mockImplementation(cb => window.setTimeout(cb, 1))
       render(<App />)
       fireEvent(
         window,
@@ -641,13 +639,11 @@ describe('App', () => {
 
       fireEvent.mouseLeave(testParamCell, { bubbles: true })
 
-      jest.advanceTimersByTime(1)
+      jest.advanceTimersByTime(CELL_TOOLTIP_DELAY)
       expect(screen.queryByRole('tooltip')).not.toBeInTheDocument()
-
-      jest.mocked(window.requestAnimationFrame).mockRestore()
     })
 
-    it('should show a tooltip with the full number on number cells', () => {
+    it('should persist a cell tooltip when it is moused into', () => {
       render(<App />)
       fireEvent(
         window,
@@ -659,13 +655,67 @@ describe('App', () => {
         })
       )
 
-      const testMetricCell = screen.getByText('1.9293')
-      fireEvent.mouseEnter(testMetricCell, { bubbles: true })
+      const testParamCell = screen.getByText(testParamStringValue)
+      fireEvent.mouseEnter(testParamCell, { bubbles: true })
 
       jest.advanceTimersByTime(CELL_TOOLTIP_DELAY)
       const tooltip = screen.getByRole('tooltip')
       expect(tooltip).toBeInTheDocument()
-      expect(tooltip).toHaveTextContent(String(testMetricNumberValue))
+
+      fireEvent.mouseLeave(testParamCell, { bubbles: true })
+      fireEvent.mouseEnter(tooltip, { bubbles: true })
+
+      jest.advanceTimersByTime(CELL_TOOLTIP_DELAY)
+      expect(tooltip).toBeInTheDocument()
+    })
+
+    it('should show the expected tooltip for all data types', () => {
+      const expectTooltipValue: (args: {
+        cellLabel: string
+        expectedTooltipResult: string
+      }) => void = ({ cellLabel, expectedTooltipResult }) => {
+        const testParamCell = screen.getByText(cellLabel)
+        expect(testParamCell).toBeInTheDocument()
+
+        fireEvent.mouseEnter(testParamCell, { bubbles: true })
+
+        jest.advanceTimersByTime(CELL_TOOLTIP_DELAY)
+        const tooltip = screen.queryByRole('tooltip')
+        expect(tooltip).toHaveTextContent(expectedTooltipResult)
+
+        fireEvent.mouseLeave(testParamCell, { bubbles: true })
+
+        jest.advanceTimersByTime(CELL_TOOLTIP_DELAY)
+        expect(screen.queryByRole('tooltip')).not.toBeInTheDocument()
+      }
+
+      render(<App />)
+      fireEvent(
+        window,
+        new MessageEvent('message', {
+          data: {
+            data: dataTypesTableData,
+            type: MessageToWebviewType.SET_DATA
+          }
+        })
+      )
+
+      expectTooltipValue({
+        cellLabel: '1.9293',
+        expectedTooltipResult: '1.9293040037155151'
+      })
+      expectTooltipValue({
+        cellLabel: 'true',
+        expectedTooltipResult: 'true'
+      })
+      expectTooltipValue({
+        cellLabel: 'false',
+        expectedTooltipResult: 'false'
+      })
+      expectTooltipValue({
+        cellLabel: '[true, false, string, 2]',
+        expectedTooltipResult: '[true, false, string, 2]'
+      })
     })
   })
 

webview/src/experiments/components/table/styles.module.scss

@@ -663,3 +663,7 @@ $badge-size: 0.85rem;
   vertical-align: middle;
   font-size: 0.5rem;
 }
+
+.cellTooltip {
+  padding: 2px 6px;
+}