Handle project having experiments with no data (#5045)

Author: mattseddon

extension/src/experiments/columns/collect/index.test.ts

@@ -73,9 +73,9 @@ describe('collectColumns', () => {
     expect(metrics).toBeDefined()
   })
 
-  it('should return an empty array if no params and metrics are provided', async () => {
+  it('should return the timestamp column if no params, metrics or deps are provided', async () => {
     const columns = await collectColumns(generateTestExpShowOutput({}))
-    expect(columns).toStrictEqual([])
+    expect(columns).toStrictEqual([timestampColumn])
   })
 
   it('should aggregate multiple different field names', async () => {

extension/src/experiments/columns/collect/index.ts

@@ -71,10 +71,7 @@ export const collectColumns = async (
   }
   await Promise.all(promises)
 
-  const columns = Object.values(acc.columns)
-  const hasNoData = isEqual(columns, [timestampColumn])
-
-  return hasNoData ? [] : columns
+  return Object.values(acc.columns)
 }
 
 export const getExpData = (expState: ExpState): ExpData | undefined => {

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

@@ -46,15 +46,15 @@ describe('ColumnsModel', () => {
   const exampleDvcRoot = 'test'
   const mockedColumnsOrderOrStatusChanged = buildMockedEventEmitter<void>()
 
-  it('should return no columns when given an output with no data', async () => {
+  it('should return the timestamp column when given output with no params, metrics or deps', async () => {
     const model = new ColumnsModel(
       '',
       buildMockMemento(),
       mockedColumnsOrderOrStatusChanged
     )
     await model.transformAndSet(generateTestExpShowOutput({}))
 
-    expect(model.getSelected()).toStrictEqual([])
+    expect(model.getSelected()).toStrictEqual([timestampColumn])
   })
 
   it('should return the expected columns when given the default output fixture', async () => {

extension/src/experiments/index.ts

@@ -481,10 +481,7 @@ export class Experiments extends BaseRepository<TableData> {
   }
 
   public getWorkspaceAndCommits() {
-    if (
-      !this.experiments.getCliError() &&
-      !this.columns.hasNonDefaultColumns()
-    ) {
+    if (!this.experiments.hasData(this.columns.hasNonDefaultColumns())) {
       return []
     }
 
@@ -496,7 +493,7 @@ export class Experiments extends BaseRepository<TableData> {
   }
 
   public getSelectedRevisions() {
-    if (!this.columns.hasNonDefaultColumns()) {
+    if (!this.experiments.hasData(this.columns.hasNonDefaultColumns())) {
       return []
     }
 
@@ -603,7 +600,7 @@ export class Experiments extends BaseRepository<TableData> {
     if (this.deferred.state === 'none') {
       return
     }
-    return this.columns.hasNonDefaultColumns()
+    return this.experiments.hasData(this.columns.hasNonDefaultColumns())
   }
 
   public getRelativeMetricsFiles() {

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

@@ -611,4 +611,38 @@ describe('ExperimentsModel', () => {
 
     expect(model.getCliError()).toBe(undefined)
   })
+
+  it('should return the correct values for hasData', () => {
+    const model = new ExperimentsModel('', buildMockMemento())
+
+    expect(model.hasData(false)).toBe(false)
+    expect(model.hasData(true)).toBe(true)
+
+    model.transformAndSetLocal(
+      outputFixture,
+      gitLogFixture,
+      { running: false },
+      rowOrderFixture,
+      { main: 6 }
+    )
+
+    expect(model.hasData(false)).toBe(true)
+    expect(model.hasData(true)).toBe(true)
+
+    model.transformAndSetLocal(
+      [
+        {
+          error: { msg: 'CLI is broken', type: 'CLI ERROR' },
+          rev: EXPERIMENT_WORKSPACE_ID
+        }
+      ],
+      gitLogFixture,
+      { running: false },
+      rowOrderFixture,
+      { main: 6 }
+    )
+
+    expect(model.hasData(false)).toBe(true)
+    expect(model.hasData(true)).toBe(true)
+  })
 })

extension/src/experiments/model/index.ts

@@ -524,6 +524,16 @@ export class ExperimentsModel extends ModelWithPersistence {
     }))
   }
 
+  public hasData(hasNonDefaultColumns: boolean) {
+    if (this.getCliError()) {
+      return true
+    }
+
+    const hasRows = this.getWorkspaceAndCommits().length > 0
+    const hasExperiments = this.getExperimentsAndQueued().length > 0
+    return (hasNonDefaultColumns || hasExperiments) && hasRows
+  }
+
   public setNbfCommitsToShow(numberOfCommitsToShow: number, branch: string) {
     this.numberOfCommitsToShow[branch] = numberOfCommitsToShow
     this.persistNbOfCommitsToShow()

extension/src/experiments/webview/messages.ts

@@ -84,10 +84,11 @@ export class WebviewMessages {
     }
     const data = await this.getWebviewData()
 
-    const hasNoRows = data.rows.length === 0
-    const hasNoData = !this.columns.hasNonDefaultColumns() || hasNoRows
+    const hasNoData = !this.experiments.hasData(
+      this.columns.hasNonDefaultColumns()
+    )
 
-    if (hasNoData && !data.cliError) {
+    if (hasNoData) {
       await commands.executeCommand(RegisteredCommands.SETUP_SHOW_EXPERIMENTS)
       return webview.dispose()
     }

extension/src/setup/index.ts

@@ -85,7 +85,6 @@ export class Setup
 
   private readonly webviewMessages: WebviewMessages
   private readonly getHasData: () => boolean | undefined
-  private readonly getExpShowError: () => string | undefined
   private readonly collectWorkspaceScale: () => Promise<WorkspaceScale>
 
   private readonly setupRun: EventEmitter<void> = this.dispose.track(
@@ -153,7 +152,6 @@ export class Setup
     void this.sendDataToWebview()
 
     this.getHasData = () => experiments.getHasData()
-    this.getExpShowError = () => experiments.getCliError()
     const onDidChangeHasData = experiments.columnsChanged.event
     this.dispose.track(onDidChangeHasData(() => this.updateProjectHasData()))
 
@@ -254,7 +252,7 @@ export class Setup
   public shouldBeShown(): { dvc: boolean; experiments: boolean } {
     return {
       dvc: !!this.getCliCompatible() && this.hasRoots(),
-      experiments: !!(this.getExpShowError() || this.getHasData())
+      experiments: !!this.getHasData()
     }
   }
 

extension/src/test/suite/setup/util.ts

@@ -26,14 +26,12 @@ export const TEMP_DIR = join(dvcDemoPath, 'temp-empty-watcher-dir')
 
 export const buildSetup = ({
   disposer,
-  cliError = undefined,
   gitVersion,
   hasData = false,
   noDvcRoot = true,
   noGitCommits = true,
   noGitRoot = true
 }: {
-  cliError?: undefined
   disposer: Disposer
   gitVersion?: string | null
   hasData?: boolean
@@ -107,7 +105,6 @@ export const buildSetup = ({
       internalCommands,
       {
         columnsChanged: mockEmitter,
-        getCliError: () => cliError,
         getHasData: () => hasData,
         isReady: () => Promise.resolve(),
         showWebview: mockShowWebview