switch from disable on running to disable on running in workspace

iterative · May 5, 2023 · cd53016 · cd53016
1 parent 7f6e90b
commit cd53016
Show file tree

Hide file tree

Showing 28 changed files with 187 additions and 119 deletions.
diff --git a/extension/package.json b/extension/package.json
diff --git a/extension/src/context.ts b/extension/src/context.ts
@@ -53,10 +53,22 @@ export class Context extends Disposable {
   }
 
   private setIsExperimentRunning(repositories: Experiments[] = []) {
+    const workspaceRunningInRunner = this.dvcRunner.isExperimentRunning()
+
+    const getContextValue = (
+      method: 'hasRunningWorkspaceExperiment' | 'hasRunningExperiment'
+    ): boolean =>
+      workspaceRunningInRunner ||
+      repositories.some(experiments => experiments[method]())
+
+    void setContextValue(
+      ContextKey.EXPERIMENT_RUNNING_WORKSPACE,
+      getContextValue('hasRunningWorkspaceExperiment')
+    )
+
     void setContextValue(
       ContextKey.EXPERIMENT_RUNNING,
-      this.dvcRunner.isExperimentRunning() ||
-        repositories.some(experiments => experiments.hasRunningExperiment())
+      getContextValue('hasRunningExperiment')
     )
   }
 }
diff --git a/extension/src/experiments/index.ts b/extension/src/experiments/index.ts
@@ -490,6 +490,10 @@ export class Experiments extends BaseRepository<TableData> {
     return this.experiments.hasRunningExperiment()
   }
 
+  public hasRunningWorkspaceExperiment() {
+    return this.experiments.hasRunningWorkspaceExperiment()
+  }
+
   public getFirstThreeColumnOrder() {
     return this.columns.getFirstThreeColumnOrder()
   }
@@ -617,7 +621,7 @@ export class Experiments extends BaseRepository<TableData> {
       this.dvcLiveOnlyCleanupInitialized = true
       void pollSignalFileForProcess(this.dvcLiveOnlySignalFile, () => {
         this.dvcLiveOnlyCleanupInitialized = false
-        if (this.hasRunningExperiment()) {
+        if (this.hasRunningWorkspaceExperiment()) {
           void this.data.update()
         }
       })

diff --git a/extension/src/experiments/model/index.ts b/extension/src/experiments/model/index.ts
@@ -166,6 +166,10 @@ export class ExperimentsModel extends ModelWithPersistence {
     return this.running.length > 0
   }
 
+  public hasRunningWorkspaceExperiment() {
+    return this.running.some(({ executor }) => executor === Executor.WORKSPACE)
+  }
+
   public hasCheckpoints() {
     return this.checkpoints
   }

diff --git a/extension/src/experiments/webview/contract.ts b/extension/src/experiments/webview/contract.ts
@@ -100,7 +100,7 @@ export type TableData = {
   hasColumns: boolean
   hasConfig: boolean
   hasMoreCommits: boolean
-  hasRunningExperiment: boolean
+  hasRunningWorkspaceExperiment: boolean
   hasValidDvcYaml: boolean
   isShowingMoreCommits: boolean
   isBranchesView: boolean

diff --git a/extension/src/experiments/webview/messages.ts b/extension/src/experiments/webview/messages.ts
@@ -281,7 +281,8 @@ export class WebviewMessages {
       hasColumns: this.columns.hasNonDefaultColumns(),
       hasConfig: this.hasConfig,
       hasMoreCommits: this.hasMoreCommits,
-      hasRunningExperiment: this.experiments.hasRunningExperiment(),
+      hasRunningWorkspaceExperiment:
+        this.experiments.hasRunningWorkspaceExperiment(),
       hasValidDvcYaml: this.hasValidDvcYaml,
       isBranchesView: this.experiments.getIsBranchesView(),
       isShowingMoreCommits: this.isShowingMoreCommits,

diff --git a/extension/src/experiments/workspace.ts b/extension/src/experiments/workspace.ts
@@ -361,7 +361,7 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews<
 
   public hasRunningExperiment() {
     return Object.values(this.repositories).some(experiments =>
-      experiments.hasRunningExperiment()
+      experiments.hasRunningWorkspaceExperiment()
     )
   }
 

diff --git a/extension/src/test/fixtures/expShow/base/tableData.ts b/extension/src/test/fixtures/expShow/base/tableData.ts
@@ -14,7 +14,7 @@ const tableDataFixture: TableData = {
   hasColumns: true,
   hasConfig: true,
   hasMoreCommits: true,
-  hasRunningExperiment: true,
+  hasRunningWorkspaceExperiment: true,
   hasValidDvcYaml: true,
   isShowingMoreCommits: true,
   isBranchesView: false,

diff --git a/extension/src/test/fixtures/expShow/dataTypes/tableData.ts b/extension/src/test/fixtures/expShow/dataTypes/tableData.ts
@@ -14,7 +14,7 @@ export const data: TableData = {
   hasColumns: true,
   hasConfig: true,
   hasMoreCommits: true,
-  hasRunningExperiment: false,
+  hasRunningWorkspaceExperiment: false,
   hasValidDvcYaml: true,
   isShowingMoreCommits: true,
   isBranchesView: false,

diff --git a/extension/src/test/fixtures/expShow/deeplyNested/tableData.ts b/extension/src/test/fixtures/expShow/deeplyNested/tableData.ts
@@ -17,7 +17,7 @@ const data: TableData = {
   hasColumns: true,
   hasConfig: true,
   hasMoreCommits: true,
-  hasRunningExperiment: false,
+  hasRunningWorkspaceExperiment: false,
   hasValidDvcYaml: true,
   isBranchesView: false,
   isShowingMoreCommits: true,

diff --git a/extension/src/test/fixtures/expShow/survival/tableData.ts b/extension/src/test/fixtures/expShow/survival/tableData.ts
@@ -14,7 +14,7 @@ const data: TableData = {
   hasColumns: true,
   hasConfig: true,
   hasMoreCommits: true,
-  hasRunningExperiment: true,
+  hasRunningWorkspaceExperiment: true,
   hasValidDvcYaml: true,
   isBranchesView: false,
   isShowingMoreCommits: true,

diff --git a/extension/src/test/suite/context.test.ts b/extension/src/test/suite/context.test.ts
@@ -67,14 +67,14 @@ suite('Context Test Suite', () => {
   const buildMockExperiments = (
     filters: FilterDefinition[] = [],
     sorts: SortDefinition[] = [],
-    experimentRunning = false,
-    queuedExperimentRunning = false
+    workspaceExperimentRunning = false,
+    experimentRunning = false
   ) => {
     return {
       getFilters: () => filters,
       getSorts: () => sorts,
       hasRunningExperiment: () => experimentRunning,
-      hasRunningQueuedExperiment: () => queuedExperimentRunning
+      hasRunningWorkspaceExperiment: () => workspaceExperimentRunning
     }
   }
 
@@ -88,7 +88,7 @@ suite('Context Test Suite', () => {
 
   // eslint-disable-next-line sonarjs/cognitive-complexity
   describe('Context', () => {
-    it('should set the dvc.experiment.running context to true whenever an experiment is running in the runner', async () => {
+    it('should set the dvc.experiment.running.workspace context to true whenever an experiment is running in the runner', async () => {
       const { executeCommandSpy, onDidStartProcess, processStarted } =
         buildContext(true)
 
@@ -101,12 +101,12 @@ suite('Context Test Suite', () => {
 
       expect(executeCommandSpy).to.be.calledWith(
         'setContext',
-        'dvc.experiment.running',
+        'dvc.experiment.running.workspace',
         true
       )
     })
 
-    it('should set the dvc.experiment.running context to true whenever the data shows an experiment is running', async () => {
+    it('should set the dvc.experiment.running.workspace context to true whenever the data shows an experiment is running', async () => {
       const {
         executeCommandSpy,
         experimentsChanged,
@@ -137,12 +137,12 @@ suite('Context Test Suite', () => {
 
       expect(executeCommandSpy).to.be.calledWith(
         'setContext',
-        'dvc.experiment.running',
+        'dvc.experiment.running.workspace',
         true
       )
     })
 
-    it('should set the dvc.experiment.running context to false whenever the runner is not running and the data shows no experiment is running', async () => {
+    it('should set the dvc.experiment.running.workspace context to false whenever the runner is not running and the data shows no experiment is running', async () => {
       const {
         executeCommandSpy,
         experimentsChanged,
@@ -164,7 +164,7 @@ suite('Context Test Suite', () => {
 
       expect(executeCommandSpy).to.be.calledWith(
         'setContext',
-        'dvc.experiment.running',
+        'dvc.experiment.running.workspace',
         false
       )
     })

diff --git a/extension/src/test/suite/experiments/index.test.ts b/extension/src/test/suite/experiments/index.test.ts
@@ -135,7 +135,7 @@ suite('Experiments Test Suite', () => {
         hasCheckpoints: true,
         hasColumns: true,
         hasConfig: true,
-        hasRunningExperiment: true,
+        hasRunningWorkspaceExperiment: true,
         hasValidDvcYaml: true,
         rows: rowsFixture,
         sorts: []
@@ -2019,7 +2019,7 @@ suite('Experiments Test Suite', () => {
 
       void experiments.setState(defaultExperimentsData)
       await dataUpdated
-      expect(experiments.hasRunningExperiment()).to.be.true
+      expect(experiments.hasRunningWorkspaceExperiment()).to.be.true
       expect(getCleanupInitialized(experiments)).to.be.true
 
       processKilled = true
@@ -2086,7 +2086,7 @@ suite('Experiments Test Suite', () => {
       await experiments.isReady()
 
       expect(
-        experimentsModel.hasRunningExperiment(),
+        experimentsModel.hasRunningWorkspaceExperiment(),
         'should have a running experiment'
       ).to.be.true
 

diff --git a/extension/src/test/suite/experiments/model/filterBy/tree.test.ts b/extension/src/test/suite/experiments/model/filterBy/tree.test.ts
@@ -110,7 +110,7 @@ suite('Experiments Filter By Tree Test Suite', () => {
         hasCheckpoints: true,
         hasColumns: true,
         hasConfig: true,
-        hasRunningExperiment: true,
+        hasRunningWorkspaceExperiment: true,
         hasValidDvcYaml: true,
         rows: filteredRows,
         sorts: []
@@ -143,7 +143,7 @@ suite('Experiments Filter By Tree Test Suite', () => {
         hasCheckpoints: true,
         hasColumns: true,
         hasConfig: true,
-        hasRunningExperiment: true,
+        hasRunningWorkspaceExperiment: true,
         hasValidDvcYaml: true,
         rows: [workspace, main],
         sorts: []
@@ -386,7 +386,7 @@ suite('Experiments Filter By Tree Test Suite', () => {
         hasCheckpoints: true,
         hasColumns: true,
         hasConfig: true,
-        hasRunningExperiment: true,
+        hasRunningWorkspaceExperiment: true,
         hasValidDvcYaml: true,
         rows: filteredRows,
         sorts: []

diff --git a/extension/src/vscode/context.ts b/extension/src/vscode/context.ts
@@ -4,10 +4,11 @@ export enum ContextKey {
   CLI_INCOMPATIBLE = 'dvc.cli.incompatible',
   COMMANDS_AVAILABLE = 'dvc.commands.available',
   EXPERIMENT_CHECKPOINTS = 'dvc.experiment.checkpoints',
-  EXPERIMENTS_WEBVIEW_ACTIVE = 'dvc.experiments.webview.active',
   EXPERIMENT_RUNNING = 'dvc.experiment.running',
+  EXPERIMENT_RUNNING_WORKSPACE = 'dvc.experiment.running.workspace',
   EXPERIMENTS_FILTERED = 'dvc.experiments.filtered',
   EXPERIMENTS_SORTED = 'dvc.experiments.sorted',
+  EXPERIMENTS_WEBVIEW_ACTIVE = 'dvc.experiments.webview.active',
   MULTIPLE_PROJECTS = 'dvc.multiple.projects',
   PARAMS_FILE_ACTIVE = 'dvc.params.file.active',
   PLOTS_WEBVIEW_ACTIVE = 'dvc.plots.webview.active',

diff --git a/webview/src/experiments/components/App.test.tsx b/webview/src/experiments/components/App.test.tsx
@@ -877,7 +877,7 @@ describe('App', () => {
       ])
     })
 
-    it('should enable the correct option for an experiment with checkpoints and close on esc', () => {
+    it('should enable the correct options for an experiment that is not running and close on esc', () => {
       renderTableWithoutRunningExperiments()
 
       const target = screen.getByText('[exp-e7a67]')
@@ -889,22 +889,41 @@ describe('App', () => {
         .filter(item => !item.className.includes('disabled'))
         .map(item => item.textContent)
       expect(itemLabels).toStrictEqual([
-        'Show Logs',
         'Apply to Workspace',
         'Create new Branch',
         'Push',
         'Modify and Run',
         'Modify and Resume',
         'Modify and Queue',
         'Star',
-        'Stop',
         'Remove'
       ])
 
       fireEvent.keyDown(menuitems[0], { bubbles: true, key: 'Escape' })
       expect(screen.queryAllByRole('menuitem')).toHaveLength(0)
     })
 
+    it('should enable the correct options for a running experiment', () => {
+      renderTable()
+
+      const target = screen.getByText('[exp-e7a67]')
+      fireEvent.contextMenu(target, { bubbles: true })
+
+      advanceTimersByTime(100)
+      const menuitems = screen.getAllByRole('menuitem')
+      const itemLabels = menuitems
+        .filter(item => !item.className.includes('disabled'))
+        .map(item => item.textContent)
+      expect(itemLabels).toStrictEqual(['Show Logs', 'Star', 'Stop'])
+
+      fireEvent.click(menuitems[0], { bubbles: true, key: 'Escape' })
+      expect(mockPostMessage).toHaveBeenCalledWith({
+        payload: 'exp-e7a67',
+        type: MessageFromWebviewType.SHOW_EXPERIMENT_LOGS
+      })
+      expect(screen.queryAllByRole('menuitem')).toHaveLength(0)
+    })
+
     it('should not close when a disabled item is clicked', () => {
       renderTableWithoutRunningExperiments()
 

diff --git a/webview/src/experiments/components/App.tsx b/webview/src/experiments/components/App.tsx
@@ -19,7 +19,7 @@ import {
   updateHasColumns,
   updateHasConfig,
   updateHasMoreCommits,
-  updateHasRunningExperiment,
+  updateHasRunningWorkspaceExperiment,
   updateHasValidDvcYaml,
   updateIsBranchesView,
   updateIsShowingMoreCommits,
@@ -76,7 +76,9 @@ export const App: React.FC<Record<string, unknown>> = () => {
                 continue
               case 'hasRunningExperiment':
                 dispatch(
-                  updateHasRunningExperiment(data.data.hasRunningExperiment)
+                  updateHasRunningWorkspaceExperiment(
+                    data.data.hasRunningWorkspaceExperiment
+                  )
                 )
                 continue
               case 'hasValidDvcYaml':

diff --git a/webview/src/experiments/components/table/body/ExperimentGroup.tsx b/webview/src/experiments/components/table/body/ExperimentGroup.tsx
@@ -6,13 +6,13 @@ import { RowProp } from '../../../util/interfaces'
 export const ExperimentGroup: React.FC<RowProp & BatchSelectionProp> = ({
   row,
   projectHasCheckpoints,
-  hasRunningExperiment,
+  hasRunningWorkspaceExperiment,
   batchRowSelection
 }) => (
   <NestedRow
     row={row}
     projectHasCheckpoints={projectHasCheckpoints}
-    hasRunningExperiment={hasRunningExperiment}
+    hasRunningWorkspaceExperiment={hasRunningWorkspaceExperiment}
     batchRowSelection={batchRowSelection}
   />
 )
diff --git a/webview/src/experiments/components/table/body/NestedRow.tsx b/webview/src/experiments/components/table/body/NestedRow.tsx
@@ -6,15 +6,15 @@ import { RowProp } from '../../../util/interfaces'
 export const NestedRow: React.FC<RowProp & BatchSelectionProp> = ({
   row,
   projectHasCheckpoints,
-  hasRunningExperiment,
+  hasRunningWorkspaceExperiment,
   batchRowSelection
 }) => {
   return (
     <RowContent
       row={row}
       className={styles.nestedRow}
       projectHasCheckpoints={projectHasCheckpoints}
-      hasRunningExperiment={hasRunningExperiment}
+      hasRunningWorkspaceExperiment={hasRunningWorkspaceExperiment}
       batchRowSelection={batchRowSelection}
     />
   )

diff --git a/webview/src/experiments/components/table/body/Row.tsx b/webview/src/experiments/components/table/body/Row.tsx
@@ -23,7 +23,7 @@ export const RowContent: React.FC<
   row,
   className,
   projectHasCheckpoints,
-  hasRunningExperiment,
+  hasRunningWorkspaceExperiment,
   batchRowSelection
 }): JSX.Element => {
   const changes = useSelector(
@@ -78,7 +78,7 @@ export const RowContent: React.FC<
         <RowContextMenu
           row={row}
           projectHasCheckpoints={projectHasCheckpoints}
-          hasRunningExperiment={hasRunningExperiment}
+          hasRunningWorkspaceExperiment={hasRunningWorkspaceExperiment}
         />
       }
     >