From 936bf1e11964ad010bbf1ec47adf850d8836eb36 Mon Sep 17 00:00:00 2001 From: mattseddon <37993418+mattseddon@users.noreply.github.com> Date: Wed, 8 Jun 2022 04:58:23 +1000 Subject: [PATCH] Use exp show data along with runner to set dvc experiment running context value (#1848) --- extension/package.json | 96 +++++------ extension/src/cli/runner.ts | 6 - extension/src/context.ts | 49 ++++++ extension/src/experiments/index.ts | 5 + .../src/experiments/model/accumulator.ts | 2 + extension/src/experiments/model/collect.ts | 11 ++ extension/src/experiments/model/index.ts | 15 +- extension/src/experiments/webview/contract.ts | 1 + extension/src/experiments/workspace.ts | 2 + extension/src/extension.ts | 9 +- .../src/test/fixtures/expShow/deeplyNested.ts | 1 + .../src/test/fixtures/expShow/tableData.ts | 1 + extension/src/test/suite/cli/runner.test.ts | 14 +- extension/src/test/suite/context.test.ts | 154 ++++++++++++++++++ .../src/test/suite/experiments/index.test.ts | 2 + .../experiments/model/filterBy/tree.test.ts | 2 + .../experiments/components/Experiments.tsx | 1 + .../components/table/Table.test.tsx | 1 + .../experiments/components/table/Table.tsx | 25 +-- webview/src/stories/Table.stories.tsx | 2 + webview/src/test/sort.ts | 1 + 21 files changed, 303 insertions(+), 97 deletions(-) create mode 100644 extension/src/context.ts create mode 100644 extension/src/test/suite/context.test.ts diff --git a/extension/package.json b/extension/package.json index c2c6f5a861..3e2dbd6990 100644 --- a/extension/package.json +++ b/extension/package.json @@ -541,11 +541,11 @@ }, { "command": "dvc.applyExperiment", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running" }, { "command": "dvc.branchExperiment", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running" }, { "command": "dvc.checkout", @@ -585,7 +585,7 @@ }, { "command": "dvc.experimentGarbageCollect", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running" }, { "command": "dvc.findInFolder", @@ -629,31 +629,31 @@ }, { "command": "dvc.modifyExperimentParamsAndQueue", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running" }, { "command": "dvc.modifyExperimentParamsAndRun", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running && !dvc.experiment.checkpoints" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running && !dvc.experiment.checkpoints" }, { "command": "dvc.modifyExperimentParamsAndResume", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running && dvc.experiment.checkpoints" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running && dvc.experiment.checkpoints" }, { "command": "dvc.modifyExperimentParamsResetAndRun", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running && dvc.experiment.checkpoints" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running && dvc.experiment.checkpoints" }, { "command": "dvc.queueExperiment", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running" }, { "command": "dvc.removeExperiment", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running" }, { "command": "dvc.removeExperimentQueue", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running" }, { "command": "dvc.removeExperimentsTableFilters", @@ -665,7 +665,7 @@ }, { "command": "dvc.removeQueuedExperiment", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running" }, { "command": "dvc.removeTarget", @@ -681,19 +681,19 @@ }, { "command": "dvc.runExperiment", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running && !dvc.experiment.checkpoints" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running && !dvc.experiment.checkpoints" }, { "command": "dvc.resumeCheckpointExperiment", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running && dvc.experiment.checkpoints" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running && dvc.experiment.checkpoints" }, { "command": "dvc.resetAndRunCheckpointExperiment", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running && dvc.experiment.checkpoints" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running && dvc.experiment.checkpoints" }, { "command": "dvc.startExperimentsQueue", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running" }, { "command": "dvc.selectForCompare", @@ -713,7 +713,7 @@ }, { "command": "dvc.stopRunningExperiment", - "when": "dvc.commands.available && dvc.project.available && dvc.runner.running" + "when": "dvc.commands.available && dvc.project.available && dvc.experiment.running" }, { "command": "dvc.views.experimentsColumnsTree.selectColumns", @@ -860,57 +860,57 @@ { "command": "dvc.runExperiment", "group": "navigation@0", - "when": "dvc.params.fileActive && !dvc.runner.running && dvc.commands.available && !dvc.experiment.checkpoints" + "when": "dvc.params.fileActive && !dvc.experiment.running && dvc.commands.available && !dvc.experiment.checkpoints" }, { "command": "dvc.runExperiment", "group": "navigation@0", - "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available && !dvc.experiment.checkpoints" + "when": "dvc.experiments.webviewActive && !dvc.experiment.running && dvc.commands.available && !dvc.experiment.checkpoints" }, { "command": "dvc.stopRunningExperiment", "group": "navigation@0", - "when": "dvc.runner.running && dvc.commands.available" + "when": "dvc.experiment.running && dvc.commands.available" }, { "command": "dvc.resumeCheckpointExperiment", "group": "navigation@1", - "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available && dvc.experiment.checkpoints" + "when": "dvc.experiments.webviewActive && !dvc.experiment.running && dvc.commands.available && dvc.experiment.checkpoints" }, { "command": "dvc.resumeCheckpointExperiment", "group": "navigation@1", - "when": "dvc.params.fileActive && !dvc.runner.running && dvc.commands.available && dvc.experiment.checkpoints" + "when": "dvc.params.fileActive && !dvc.experiment.running && dvc.commands.available && dvc.experiment.checkpoints" }, { "command": "dvc.resetAndRunCheckpointExperiment", "group": "navigation@2", - "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available && dvc.experiment.checkpoints" + "when": "dvc.experiments.webviewActive && !dvc.experiment.running && dvc.commands.available && dvc.experiment.checkpoints" }, { "command": "dvc.resetAndRunCheckpointExperiment", "group": "navigation@2", - "when": "dvc.params.fileActive && !dvc.runner.running && dvc.commands.available && dvc.experiment.checkpoints" + "when": "dvc.params.fileActive && !dvc.experiment.running && dvc.commands.available && dvc.experiment.checkpoints" }, { "command": "dvc.startExperimentsQueue", "group": "navigation@3", - "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available" + "when": "dvc.experiments.webviewActive && !dvc.experiment.running && dvc.commands.available" }, { "command": "dvc.startExperimentsQueue", "group": "navigation@3", - "when": "dvc.params.fileActive && !dvc.runner.running && dvc.commands.available" + "when": "dvc.params.fileActive && !dvc.experiment.running && dvc.commands.available" }, { "command": "dvc.queueExperiment", "group": "navigation@4", - "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available" + "when": "dvc.experiments.webviewActive && !dvc.experiment.running && dvc.commands.available" }, { "command": "dvc.queueExperiment", "group": "navigation@4", - "when": "dvc.params.fileActive && !dvc.runner.running && dvc.commands.available" + "when": "dvc.params.fileActive && !dvc.experiment.running && dvc.commands.available" } ], "view/item/context": [ @@ -1007,37 +1007,37 @@ { "command": "dvc.views.experimentsTree.applyExperiment", "group": "inline@1", - "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(checkpoint|experiment)$/ && !dvc.runner.running" + "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(checkpoint|experiment)$/ && !dvc.experiment.running" }, { "command": "dvc.views.experimentsTree.branchExperiment", "group": "inline@2", - "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(checkpoint|experiment)$/ && !dvc.runner.running" + "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(checkpoint|experiment)$/ && !dvc.experiment.running" }, { "command": "dvc.views.experimentsTree.removeExperiment", "group": "inline@3", - "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(experiment|queued)$/ && !dvc.runner.running" + "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(experiment|queued)$/ && !dvc.experiment.running" }, { "command": "dvc.views.experimentsTree.runExperiment", "group": "1_modify@1", - "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.runner.running && !dvc.experiment.checkpoints" + "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.experiment.running && !dvc.experiment.checkpoints" }, { "command": "dvc.views.experimentsTree.resumeCheckpointExperiment", "group": "1_modify@1", - "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.runner.running && dvc.experiment.checkpoints" + "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.experiment.running && dvc.experiment.checkpoints" }, { "command": "dvc.views.experimentsTree.resetAndRunCheckpointExperiment", "group": "1_modify@2", - "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.runner.running && dvc.experiment.checkpoints" + "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.experiment.running && dvc.experiment.checkpoints" }, { "command": "dvc.views.experimentsTree.queueExperiment", "group": "1_modify@3", - "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.runner.running" + "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.experiment.running" }, { "command": "dvc.views.experimentsTree.selectExperiments", @@ -1093,42 +1093,42 @@ }, { "command": "dvc.runExperiment", - "when": "view == dvc.views.experimentsTree && !dvc.experiments.webviewActive && !dvc.runner.running && !dvc.experiment.checkpoints", + "when": "view == dvc.views.experimentsTree && !dvc.experiments.webviewActive && !dvc.experiment.running && !dvc.experiment.checkpoints", "group": "1_run@1" }, { "command": "dvc.resumeCheckpointExperiment", - "when": "view == dvc.views.experimentsTree && !dvc.experiments.webviewActive && !dvc.runner.running && dvc.experiment.checkpoints", + "when": "view == dvc.views.experimentsTree && !dvc.experiments.webviewActive && !dvc.experiment.running && dvc.experiment.checkpoints", "group": "1_run@1" }, { "command": "dvc.resetAndRunCheckpointExperiment", - "when": "view == dvc.views.experimentsTree && !dvc.experiments.webviewActive && !dvc.runner.running && dvc.experiment.checkpoints", + "when": "view == dvc.views.experimentsTree && !dvc.experiments.webviewActive && !dvc.experiment.running && dvc.experiment.checkpoints", "group": "1_run@2" }, { "command": "dvc.runExperiment", - "when": "view == dvc.views.experimentsTree && dvc.experiments.webviewActive && !dvc.runner.running && !dvc.experiment.checkpoints", + "when": "view == dvc.views.experimentsTree && dvc.experiments.webviewActive && !dvc.experiment.running && !dvc.experiment.checkpoints", "group": "navigation@1" }, { "command": "dvc.resumeCheckpointExperiment", - "when": "view == dvc.views.experimentsTree && dvc.experiments.webviewActive && !dvc.runner.running && dvc.experiment.checkpoints", + "when": "view == dvc.views.experimentsTree && dvc.experiments.webviewActive && !dvc.experiment.running && dvc.experiment.checkpoints", "group": "navigation@1" }, { "command": "dvc.resetAndRunCheckpointExperiment", - "when": "view == dvc.views.experimentsTree && dvc.experiments.webviewActive && !dvc.runner.running && dvc.experiment.checkpoints", + "when": "view == dvc.views.experimentsTree && dvc.experiments.webviewActive && !dvc.experiment.running && dvc.experiment.checkpoints", "group": "navigation@2" }, { "command": "dvc.stopRunningExperiment", - "when": "view == dvc.views.experimentsTree && !dvc.experiments.webviewActive && dvc.runner.running", + "when": "view == dvc.views.experimentsTree && !dvc.experiments.webviewActive && dvc.experiment.running", "group": "1_run@1" }, { "command": "dvc.stopRunningExperiment", - "when": "view == dvc.views.experimentsTree && dvc.experiments.webviewActive && dvc.runner.running", + "when": "view == dvc.views.experimentsTree && dvc.experiments.webviewActive && dvc.experiment.running", "group": "navigation@1" }, { @@ -1153,32 +1153,32 @@ }, { "command": "dvc.modifyExperimentParamsAndRun", - "when": "view == dvc.views.experimentsTree && !dvc.runner.running && !dvc.experiment.checkpoints", + "when": "view == dvc.views.experimentsTree && !dvc.experiment.running && !dvc.experiment.checkpoints", "group": "2_modify@1" }, { "command": "dvc.modifyExperimentParamsAndResume", - "when": "view == dvc.views.experimentsTree && !dvc.runner.running && dvc.experiment.checkpoints", + "when": "view == dvc.views.experimentsTree && !dvc.experiment.running && dvc.experiment.checkpoints", "group": "2_modify@1" }, { "command": "dvc.modifyExperimentParamsResetAndRun", - "when": "view == dvc.views.experimentsTree && !dvc.runner.running && dvc.experiment.checkpoints", + "when": "view == dvc.views.experimentsTree && !dvc.experiment.running && dvc.experiment.checkpoints", "group": "2_modify@2" }, { "command": "dvc.modifyExperimentParamsAndQueue", - "when": "view == dvc.views.experimentsTree && !dvc.runner.running", + "when": "view == dvc.views.experimentsTree && !dvc.experiment.running", "group": "2_modify@3" }, { "command": "dvc.queueExperiment", - "when": "view == dvc.views.experimentsTree && !dvc.runner.running", + "when": "view == dvc.views.experimentsTree && !dvc.experiment.running", "group": "3_queue@1" }, { "command": "dvc.startExperimentsQueue", - "when": "view == dvc.views.experimentsTree && !dvc.runner.running", + "when": "view == dvc.views.experimentsTree && !dvc.experiment.running", "group": "3_queue@2" }, { diff --git a/extension/src/cli/runner.ts b/extension/src/cli/runner.ts index 1922257664..12a1f622a2 100644 --- a/extension/src/cli/runner.ts +++ b/extension/src/cli/runner.ts @@ -10,7 +10,6 @@ import { getOptions } from './options' import { Config } from '../config' import { PseudoTerminal } from '../vscode/pseudoTerminal' import { createProcess, Process } from '../processExecution' -import { setContextValue } from '../vscode/context' import { StopWatch } from '../util/time' import { sendErrorTelemetryEvent, sendTelemetryEvent } from '../telemetry' import { EventName } from '../telemetry/constants' @@ -70,7 +69,6 @@ export class CliRunner extends Disposable implements ICli { this.dispose.track( this.onDidCompleteProcess(() => { this.pseudoTerminal.setBlocked(false) - CliRunner.setRunningContext(false) this.processOutput.fire( '\r\nTerminal will be reused by DVC, press any key to close it\r\n\n' ) @@ -101,9 +99,6 @@ export class CliRunner extends Disposable implements ICli { ) } - private static setRunningContext = (isRunning: boolean) => - setContextValue('dvc.runner.running', isRunning) - public runExperiment(dvcRoot: string, ...args: Args) { return this.run( dvcRoot, @@ -207,7 +202,6 @@ export class CliRunner extends Disposable implements ICli { } private startProcess(cwd: string, args: Args) { - CliRunner.setRunningContext(true) this.pseudoTerminal.setBlocked(true) this.processOutput.fire(`Running: dvc ${args.join(' ')}\r\n\n`) this.currentProcess = this.createProcess({ diff --git a/extension/src/context.ts b/extension/src/context.ts new file mode 100644 index 0000000000..14e5b75130 --- /dev/null +++ b/extension/src/context.ts @@ -0,0 +1,49 @@ +import { Disposable } from './class/dispose' +import { CliRunner } from './cli/runner' +import { WorkspaceExperiments } from './experiments/workspace' +import { setContextValue } from './vscode/context' + +export class Context extends Disposable { + private readonly experiments: WorkspaceExperiments + private readonly cliRunner: CliRunner + + constructor(experiments: WorkspaceExperiments, cliRunner: CliRunner) { + super() + + this.experiments = experiments + this.cliRunner = cliRunner + + this.dispose.track( + this.cliRunner.onDidStartProcess(() => { + this.setIsExperimentRunning() + }) + ) + + this.dispose.track( + this.cliRunner.onDidCompleteProcess(({ cwd }) => + this.experiments.getRepository(cwd).update() + ) + ) + + this.dispose.track( + this.experiments.onDidChangeExperiments(() => { + this.setIsExperimentRunning() + }) + ) + } + + private setIsExperimentRunning() { + if (this.cliRunner.isExperimentRunning()) { + setContextValue('dvc.experiment.running', true) + return + } + + let hasRunning = false + for (const dvcRoot of this.experiments.getDvcRoots()) { + if (this.experiments.getRepository(dvcRoot).hasRunningExperiment()) { + hasRunning = true + } + } + setContextValue('dvc.experiment.running', hasRunning) + } +} diff --git a/extension/src/experiments/index.ts b/extension/src/experiments/index.ts index 7c9a71e0cf..34ddf474a8 100644 --- a/extension/src/experiments/index.ts +++ b/extension/src/experiments/index.ts @@ -377,6 +377,10 @@ export class Experiments extends BaseRepository { } } + public hasRunningExperiment() { + return this.experiments.hasRunningExperiment() + } + private hideTableColumn(path: string) { this.toggleColumnStatus(path) sendTelemetryEvent( @@ -438,6 +442,7 @@ export class Experiments extends BaseRepository { columns: this.columns.getSelected(), hasCheckpoints: this.hasCheckpoints(), hasColumns: this.columns.hasColumns(), + hasRunningExperiment: this.experiments.hasRunningExperiment(), rows: this.experiments.getRowData(), sorts: this.experiments.getSorts() } diff --git a/extension/src/experiments/model/accumulator.ts b/extension/src/experiments/model/accumulator.ts index 93d5f9d5c3..7a5c8f5fe3 100644 --- a/extension/src/experiments/model/accumulator.ts +++ b/extension/src/experiments/model/accumulator.ts @@ -5,10 +5,12 @@ export class ExperimentsAccumulator { public branches: Experiment[] = [] public checkpointsByTip: Map = new Map() public experimentsByBranch: Map = new Map() + public hasRunning: boolean constructor(workspace: Experiment | undefined) { if (workspace) { this.workspace = workspace } + this.hasRunning = !!workspace?.running } } diff --git a/extension/src/experiments/model/collect.ts b/extension/src/experiments/model/collect.ts index 4301c2afb6..bded28af74 100644 --- a/extension/src/experiments/model/collect.ts +++ b/extension/src/experiments/model/collect.ts @@ -192,6 +192,15 @@ const transformExperimentOrCheckpointData = ( } } +const collectHasRunningExperiment = ( + acc: ExperimentsAccumulator, + experiment: Experiment +) => { + if (experiment.running) { + acc.hasRunning = true + } +} + const collectExperimentOrCheckpoint = ( acc: ExperimentsAccumulator, experiment: Experiment, @@ -227,6 +236,7 @@ const collectFromExperimentsObject = ( } collectExperimentOrCheckpoint(acc, experiment, branchName, checkpointTipId) + collectHasRunningExperiment(acc, experiment) } } @@ -248,6 +258,7 @@ const collectFromBranchesObject = ( if (branch) { collectFromExperimentsObject(acc, experimentsObject, sha, branch.label) + collectHasRunningExperiment(acc, branch) acc.branches.push(branch) } diff --git a/extension/src/experiments/model/index.ts b/extension/src/experiments/model/index.ts index 33995acddf..d191bb0d45 100644 --- a/extension/src/experiments/model/index.ts +++ b/extension/src/experiments/model/index.ts @@ -56,6 +56,7 @@ export class ExperimentsModel extends ModelWithPersistence { private useFiltersForSelection = false private currentSorts: SortDefinition[] + private hasRunning = false constructor(dvcRoot: string, workspaceState: Memento) { super(dvcRoot, workspaceState) @@ -85,13 +86,19 @@ export class ExperimentsModel extends ModelWithPersistence { } public transformAndSet(data: ExperimentsOutput) { - const { workspace, branches, experimentsByBranch, checkpointsByTip } = - collectExperiments(data) + const { + workspace, + branches, + experimentsByBranch, + checkpointsByTip, + hasRunning + } = collectExperiments(data) this.workspace = workspace this.branches = branches this.experimentsByBranch = experimentsByBranch this.checkpointsByTip = checkpointsByTip + this.hasRunning = hasRunning this.setColoredStatus() } @@ -117,6 +124,10 @@ export class ExperimentsModel extends ModelWithPersistence { return this.coloredStatus[id] } + public hasRunningExperiment() { + return this.hasRunning + } + public canSelect() { return canSelect(this.coloredStatus) } diff --git a/extension/src/experiments/webview/contract.ts b/extension/src/experiments/webview/contract.ts index f6730718bf..47804cc362 100644 --- a/extension/src/experiments/webview/contract.ts +++ b/extension/src/experiments/webview/contract.ts @@ -57,6 +57,7 @@ export type TableData = { columnWidths: Record hasCheckpoints: boolean hasColumns: boolean + hasRunningExperiment: boolean rows: Row[] sorts: SortDefinition[] } diff --git a/extension/src/experiments/workspace.ts b/extension/src/experiments/workspace.ts index adce89ffbd..a8999f3c09 100644 --- a/extension/src/experiments/workspace.ts +++ b/extension/src/experiments/workspace.ts @@ -19,6 +19,8 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews< new EventEmitter() ) + public readonly onDidChangeExperiments = this.experimentsChanged.event + public readonly columnsChanged = this.dispose.track(new EventEmitter()) public readonly updatesPaused: EventEmitter diff --git a/extension/src/extension.ts b/extension/src/extension.ts index 1a244afcb9..47467711be 100644 --- a/extension/src/extension.ts +++ b/extension/src/extension.ts @@ -1,8 +1,9 @@ import { commands, env, Event, EventEmitter, ExtensionContext } from 'vscode' -import { Config } from './config' import { CliExecutor } from './cli/executor' import { CliRunner } from './cli/runner' import { CliReader } from './cli/reader' +import { Config } from './config' +import { Context } from './context' import { isVersionCompatible } from './cli/version' import { isPythonExtensionInstalled } from './extensions/python' import { WorkspaceExperiments } from './experiments/workspace' @@ -133,11 +134,7 @@ export class Extension extends Disposable implements IExtension { new WorkspaceRepositories(this.internalCommands) ) - this.dispose.track( - this.cliRunner.onDidCompleteProcess(({ cwd }) => - this.experiments.getRepository(cwd).update() - ) - ) + this.dispose.track(new Context(this.experiments, this.cliRunner)) this.dispose.track( new ExperimentsColumnsTree( diff --git a/extension/src/test/fixtures/expShow/deeplyNested.ts b/extension/src/test/fixtures/expShow/deeplyNested.ts index 54392b3756..4fe2131f31 100644 --- a/extension/src/test/fixtures/expShow/deeplyNested.ts +++ b/extension/src/test/fixtures/expShow/deeplyNested.ts @@ -282,6 +282,7 @@ const deeplyNestedTableData: TableData = { columnOrder: [], columnWidths: {}, hasCheckpoints: false, + hasRunningExperiment: false, sorts: [ { path: 'params:params.yaml:nested1.doubled', diff --git a/extension/src/test/fixtures/expShow/tableData.ts b/extension/src/test/fixtures/expShow/tableData.ts index a190bc8a48..b3f2f6be02 100644 --- a/extension/src/test/fixtures/expShow/tableData.ts +++ b/extension/src/test/fixtures/expShow/tableData.ts @@ -6,6 +6,7 @@ const tableDataFixture: TableData = { rows: rowsFixture, columns: columnsFixture, hasCheckpoints: true, + hasRunningExperiment: true, hasColumns: true, sorts: [], changes: [], diff --git a/extension/src/test/suite/cli/runner.test.ts b/extension/src/test/suite/cli/runner.test.ts index 28c2cb1fe3..df4ea6153a 100644 --- a/extension/src/test/suite/cli/runner.test.ts +++ b/extension/src/test/suite/cli/runner.test.ts @@ -1,7 +1,7 @@ import { afterEach, beforeEach, describe, it, suite } from 'mocha' import { expect } from 'chai' import { restore, spy, stub } from 'sinon' -import { window, commands, Event, EventEmitter } from 'vscode' +import { window, Event, EventEmitter } from 'vscode' import { Disposable, Disposer } from '../../../extension' import { Config } from '../../../config' import { CliRunner } from '../../../cli/runner' @@ -38,8 +38,6 @@ suite('CLI Runner Test Suite', () => { const cliRunner = disposable.track(new CliRunner({} as Config, 'sleep')) const cwd = __dirname - const executeCommandSpy = spy(commands, 'executeCommand') - const onDidCompleteProcess = (): Promise => new Promise(resolve => disposable.track(cliRunner.onDidCompleteProcess(() => resolve())) @@ -50,11 +48,6 @@ suite('CLI Runner Test Suite', () => { const completed = onDidCompleteProcess() expect(cliRunner.isExperimentRunning()).to.be.true - expect(executeCommandSpy).to.be.calledWith( - 'setContext', - 'dvc.runner.running', - true - ) // eslint-disable-next-line @typescript-eslint/no-explicit-any const closeSpy = spy((cliRunner as any).pseudoTerminal, 'close') @@ -65,11 +58,6 @@ suite('CLI Runner Test Suite', () => { await completed expect(cliRunner.isExperimentRunning()).to.be.false - expect(executeCommandSpy).to.be.calledWith( - 'setContext', - 'dvc.runner.running', - false - ) }).timeout(WEBVIEW_TEST_TIMEOUT) it('should be able to execute a command and provide the correct events in the correct order', async () => { diff --git a/extension/src/test/suite/context.test.ts b/extension/src/test/suite/context.test.ts new file mode 100644 index 0000000000..a077b29b19 --- /dev/null +++ b/extension/src/test/suite/context.test.ts @@ -0,0 +1,154 @@ +import { resolve } from 'path' +import { Disposable } from '@hediet/std/disposable' +import { afterEach, beforeEach, describe, it, suite } from 'mocha' +import { expect } from 'chai' +import { restore, spy, stub } from 'sinon' +import { commands, EventEmitter } from 'vscode' +import { CliRunner } from '../../cli/runner' +import { WorkspaceExperiments } from '../../experiments/workspace' +import { Context } from '../../context' + +suite('Context Test Suite', () => { + const disposable = Disposable.fn() + + const buildContext = (runnerRunning: boolean) => { + const executeCommandSpy = spy(commands, 'executeCommand') + + const processStarted = disposable.track(new EventEmitter()) + const processCompleted = disposable.track(new EventEmitter()) + const onDidStartProcess = processStarted.event + const onDidCompleteProcess = processCompleted.event + + const experimentsChanged = disposable.track(new EventEmitter()) + const onDidChangeExperiments = experimentsChanged.event + + const mockGetDvcRoots = stub().returns([]) + const mockGetRepository = stub().returns({ + update: stub() + }) + + const mockCliRunner = { + isExperimentRunning: () => runnerRunning, + onDidCompleteProcess, + onDidStartProcess + } as unknown as CliRunner + const mockExperiments = { + getDvcRoots: mockGetDvcRoots, + getRepository: mockGetRepository, + onDidChangeExperiments + } as unknown as WorkspaceExperiments + + const context = disposable.track( + new Context(mockExperiments, mockCliRunner) + ) + + return { + context, + executeCommandSpy, + experimentsChanged, + mockCliRunner, + mockExperiments, + mockGetDvcRoots, + mockGetRepository, + onDidChangeExperiments, + onDidCompleteProcess, + onDidStartProcess, + processCompleted, + processStarted + } + } + + beforeEach(() => { + restore() + }) + + afterEach(() => { + disposable.dispose() + }) + + // 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 () => { + const { executeCommandSpy, onDidStartProcess, processStarted } = + buildContext(true) + + const processStartedEvent = new Promise(resolve => + disposable.track(onDidStartProcess(() => resolve(undefined))) + ) + + processStarted.fire() + await processStartedEvent + + expect(executeCommandSpy).to.be.calledWith( + 'setContext', + 'dvc.experiment.running', + true + ) + }) + + it('should set the dvc.experiment.running context to true whenever the data shows an experiment is running', async () => { + const { + executeCommandSpy, + experimentsChanged, + mockGetDvcRoots, + mockGetRepository, + onDidChangeExperiments + } = buildContext(false) + + const experimentsChangedEvent = new Promise(resolve => + disposable.track(onDidChangeExperiments(() => resolve(undefined))) + ) + + const mockDvcRoot = resolve('first', 'root') + const mockOtherDvcRoot = resolve('second', 'root') + + mockGetDvcRoots.returns([mockDvcRoot, mockOtherDvcRoot]) + mockGetRepository.callsFake(dvcRoot => { + if (dvcRoot === mockDvcRoot) { + return { hasRunningExperiment: () => true } + } + if (dvcRoot === mockOtherDvcRoot) { + return { hasRunningExperiment: () => false } + } + }) + + experimentsChanged.fire() + await experimentsChangedEvent + + expect(executeCommandSpy).to.be.calledWith( + 'setContext', + 'dvc.experiment.running', + 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 () => { + const { + executeCommandSpy, + experimentsChanged, + mockGetDvcRoots, + mockGetRepository, + onDidChangeExperiments + } = buildContext(false) + + const experimentsChangedEvent = new Promise(resolve => + disposable.track(onDidChangeExperiments(() => resolve(undefined))) + ) + + const mockDvcRoot = resolve('first', 'root') + mockGetDvcRoots.returns([mockDvcRoot]) + mockGetRepository.callsFake(() => { + return { hasRunningExperiment: () => false } + }) + + experimentsChanged.fire() + await experimentsChangedEvent + + expect(executeCommandSpy).to.be.calledWith( + 'setContext', + 'dvc.experiment.running', + false + ) + }) + }) +}) diff --git a/extension/src/test/suite/experiments/index.test.ts b/extension/src/test/suite/experiments/index.test.ts index 92a6db05f0..ec617dc378 100644 --- a/extension/src/test/suite/experiments/index.test.ts +++ b/extension/src/test/suite/experiments/index.test.ts @@ -127,6 +127,7 @@ suite('Experiments Test Suite', () => { columns: columnsFixture, hasCheckpoints: true, hasColumns: true, + hasRunningExperiment: true, rows: rowsFixture, sorts: [] } @@ -684,6 +685,7 @@ suite('Experiments Test Suite', () => { columns: [], hasCheckpoints: true, hasColumns: true, + hasRunningExperiment: true, rows: rowsFixture, sorts: [] } diff --git a/extension/src/test/suite/experiments/model/filterBy/tree.test.ts b/extension/src/test/suite/experiments/model/filterBy/tree.test.ts index 0a1b6f35c1..b46814b50c 100644 --- a/extension/src/test/suite/experiments/model/filterBy/tree.test.ts +++ b/extension/src/test/suite/experiments/model/filterBy/tree.test.ts @@ -111,6 +111,7 @@ suite('Experiments Filter By Tree Test Suite', () => { columns: columnsFixture, hasCheckpoints: true, hasColumns: true, + hasRunningExperiment: true, rows: filteredRows, sorts: [] } @@ -139,6 +140,7 @@ suite('Experiments Filter By Tree Test Suite', () => { columns: columnsFixture, hasCheckpoints: true, hasColumns: true, + hasRunningExperiment: true, rows: [workspace, main], sorts: [] } diff --git a/webview/src/experiments/components/Experiments.tsx b/webview/src/experiments/components/Experiments.tsx index 964440a785..285f09dae9 100644 --- a/webview/src/experiments/components/Experiments.tsx +++ b/webview/src/experiments/components/Experiments.tsx @@ -140,6 +140,7 @@ export const ExperimentsTable: React.FC<{ columns: [], hasCheckpoints: false, hasColumns: false, + hasRunningExperiment: false, rows: [], sorts: [], ...initiallyUndefinedTableData diff --git a/webview/src/experiments/components/table/Table.test.tsx b/webview/src/experiments/components/table/Table.test.tsx index b72a37d6ec..14f63b0a37 100644 --- a/webview/src/experiments/components/table/Table.test.tsx +++ b/webview/src/experiments/components/table/Table.test.tsx @@ -114,6 +114,7 @@ describe('Table', () => { columns: [], hasCheckpoints: false, hasColumns: true, + hasRunningExperiment: false, rows: [], sorts: [] } diff --git a/webview/src/experiments/components/table/Table.tsx b/webview/src/experiments/components/table/Table.tsx index 6c7d144e5e..eb294032bb 100644 --- a/webview/src/experiments/components/table/Table.tsx +++ b/webview/src/experiments/components/table/Table.tsx @@ -1,6 +1,5 @@ import React from 'react' import cx from 'classnames' -import { Row } from 'dvc/src/experiments/webview/contract' import styles from './styles.module.scss' import { TableHead } from './TableHead' import { RowContent } from './Row' @@ -102,26 +101,8 @@ export const Table: React.FC = ({ tableData }) => { const { getTableProps, rows } = instance - const { - sorts, - columns, - changes, - rows: experimentRows, - hasCheckpoints - } = tableData - - const someExperimentsRunning = React.useMemo(() => { - const findRunningExperiment: (experiments?: Row[]) => boolean = ( - experiments?: Row[] - ) => { - return !!experiments?.find( - experiment => - experiment.running || findRunningExperiment(experiment.subRows) - ) - } - - return findRunningExperiment(experimentRows) - }, [experimentRows]) + const { sorts, columns, changes, hasCheckpoints, hasRunningExperiment } = + tableData return (
@@ -133,7 +114,7 @@ export const Table: React.FC = ({ instance={instance} key={row.id} changes={changes} - contextMenuDisabled={someExperimentsRunning} + contextMenuDisabled={hasRunningExperiment} projectHasCheckpoints={hasCheckpoints} /> ))} diff --git a/webview/src/stories/Table.stories.tsx b/webview/src/stories/Table.stories.tsx index 6074f613fd..c426155bb5 100644 --- a/webview/src/stories/Table.stories.tsx +++ b/webview/src/stories/Table.stories.tsx @@ -19,6 +19,7 @@ const tableData: TableData = { columns: columnsFixture, hasCheckpoints: true, hasColumns: true, + hasRunningExperiment: true, rows: rowsFixture.map(row => ({ ...row, subRows: row.subRows?.map(experiment => ({ @@ -59,6 +60,7 @@ export const WithNoRunningExperiments = Template.bind({}) WithNoRunningExperiments.args = { tableData: { ...tableData, + hasRunningExperiment: false, rows: rowsFixture.map(row => ({ ...row, running: false, diff --git a/webview/src/test/sort.ts b/webview/src/test/sort.ts index b464bdad59..6df8195de6 100644 --- a/webview/src/test/sort.ts +++ b/webview/src/test/sort.ts @@ -37,6 +37,7 @@ export const tableData: TableData = { columns, hasCheckpoints: false, hasColumns: true, + hasRunningExperiment: false, rows: [ { id: 'workspace',