From 6d2977d69bea140941108f4c9b19681ca97ae0bc Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Thu, 19 May 2022 12:39:33 +1000 Subject: [PATCH 1/5] add context value for whether the workspace has checkpoints --- extension/src/experiments/index.ts | 14 +++++++++-- extension/src/experiments/workspace.test.ts | 4 ++- extension/src/experiments/workspace.ts | 28 ++++++++++++++++++++- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/extension/src/experiments/index.ts b/extension/src/experiments/index.ts index 6bca97a76f..f862d5cb0f 100644 --- a/extension/src/experiments/index.ts +++ b/extension/src/experiments/index.ts @@ -44,6 +44,7 @@ export const ExperimentsScale = { export class Experiments extends BaseRepository { public readonly onDidChangeExperiments: Event public readonly onDidChangeColumns: Event + public readonly onDidChangeCheckpoints: Event public readonly viewKey = ViewKey.EXPERIMENTS @@ -58,6 +59,10 @@ export class Experiments extends BaseRepository { new EventEmitter() ) + private readonly checkpointsChanged = this.dispose.track( + new EventEmitter() + ) + private readonly columnsChanged = this.dispose.track(new EventEmitter()) private readonly internalCommands: InternalCommands @@ -77,6 +82,7 @@ export class Experiments extends BaseRepository { this.onDidChangeExperiments = this.experimentsChanged.event this.onDidChangeColumns = this.columnsChanged.event + this.onDidChangeCheckpoints = this.checkpointsChanged.event this.experiments = this.dispose.track( new ExperimentsModel(dvcRoot, workspaceState) @@ -96,9 +102,13 @@ export class Experiments extends BaseRepository { this.dispose.track(this.cliData.onDidUpdate(data => this.setState(data))) this.dispose.track( - this.fileSystemData.onDidUpdate(data => + this.fileSystemData.onDidUpdate(data => { + const hadCheckpoints = this.hasCheckpoints() this.checkpoints.transformAndSet(data) - ) + if (hadCheckpoints !== this.hasCheckpoints()) { + this.checkpointsChanged.fire() + } + }) ) this.handleMessageFromWebview() diff --git a/extension/src/experiments/workspace.test.ts b/extension/src/experiments/workspace.test.ts index 1e47b59560..aa4f85a86a 100644 --- a/extension/src/experiments/workspace.test.ts +++ b/extension/src/experiments/workspace.test.ts @@ -23,6 +23,7 @@ const mockedGetInput = jest.mocked(getInput) const mockedRun = jest.fn() const mockedExpFunc = jest.fn() +jest.mock('vscode') jest.mock('@hediet/std/disposable') jest.mock('../vscode/quickPick') jest.mock('../vscode/inputBox') @@ -69,7 +70,8 @@ describe('Experiments', () => { pickCurrentExperiment: jest.fn(), showWebview: jest.fn() } as unknown as Experiments - } + }, + buildMockedEventEmitter() ) describe('getCwdThenReport', () => { diff --git a/extension/src/experiments/workspace.ts b/extension/src/experiments/workspace.ts index 2c9fa08c85..1dda47afe9 100644 --- a/extension/src/experiments/workspace.ts +++ b/extension/src/experiments/workspace.ts @@ -8,6 +8,7 @@ import { getInput } from '../vscode/inputBox' import { BaseWorkspaceWebviews } from '../webview/workspace' import { WorkspacePlots } from '../plots/workspace' import { Title } from '../vscode/title' +import { setContextValue } from '../vscode/context' export class WorkspaceExperiments extends BaseWorkspaceWebviews< Experiments, @@ -21,15 +22,34 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews< public readonly updatesPaused: EventEmitter + private readonly checkpointsChanged: EventEmitter + constructor( internalCommands: InternalCommands, updatesPaused: EventEmitter, workspaceState: Memento, - experiments?: Record + experiments?: Record, + checkpointsChanged?: EventEmitter ) { super(internalCommands, workspaceState, experiments) this.updatesPaused = updatesPaused + + this.checkpointsChanged = this.dispose.track( + checkpointsChanged || new EventEmitter() + ) + + const onDidChangeCheckpoints = this.checkpointsChanged.event + + this.dispose.track( + onDidChangeCheckpoints(() => { + const workspaceHasCheckpoints = Object.values(this.repositories).some( + experiments => experiments.hasCheckpoints() + ) + + setContextValue('dvc.experiment.checkpoints', workspaceHasCheckpoints) + }) + ) } public linkRepositories(workspacePlots: WorkspacePlots) { @@ -224,6 +244,12 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews< }) ) + experiments.dispose.track( + experiments.onDidChangeCheckpoints(() => { + this.checkpointsChanged.fire() + }) + ) + return experiments } From 7e69bf332c81490601dcd11530255c5c3d3ed67f Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Thu, 19 May 2022 14:39:24 +1000 Subject: [PATCH 2/5] update all user facing commands and context menus --- extension/package.json | 206 ++++++++++-------- extension/package.nls.json | 9 +- extension/resources/dark/run-all.svg | 1 - extension/resources/dark/run-experiment.svg | 1 - extension/resources/dark/stop-experiment.svg | 1 - extension/resources/light/run-all.svg | 1 - extension/resources/light/run-experiment.svg | 1 - extension/resources/light/stop-experiment.svg | 1 - extension/src/cli/runner.ts | 2 +- extension/src/commands/external.ts | 7 +- .../src/experiments/commands/register.ts | 28 ++- extension/src/experiments/index.ts | 2 +- extension/src/experiments/model/tree.ts | 23 +- extension/src/telemetry/constants.ts | 7 +- .../src/test/suite/experiments/index.test.ts | 2 +- .../test/suite/experiments/model/tree.test.ts | 2 +- .../test/suite/experiments/workspace.test.ts | 70 +++++- 17 files changed, 238 insertions(+), 126 deletions(-) delete mode 100644 extension/resources/dark/run-all.svg delete mode 100644 extension/resources/dark/run-experiment.svg delete mode 100644 extension/resources/dark/stop-experiment.svg delete mode 100644 extension/resources/light/run-all.svg delete mode 100644 extension/resources/light/run-experiment.svg delete mode 100644 extension/resources/light/stop-experiment.svg diff --git a/extension/package.json b/extension/package.json index 4a68365c11..3f7e712f67 100644 --- a/extension/package.json +++ b/extension/package.json @@ -229,23 +229,23 @@ "light": "resources/light/queue-experiment.svg" } }, + { + "title": "%command.modifyExperimentParamsAndResume%", + "command": "dvc.modifyExperimentParamsAndResume", + "category": "DVC", + "icon": "$(play)" + }, { "title": "%command.modifyExperimentParamsAndRun%", "command": "dvc.modifyExperimentParamsAndRun", "category": "DVC", - "icon": { - "light": "resources/light/run-experiment.svg", - "dark": "resources/dark/run-experiment.svg" - } + "icon": "$(play)" }, { "title": "%command.modifyExperimentParamsResetAndRun%", "command": "dvc.modifyExperimentParamsResetAndRun", "category": "DVC", - "icon": { - "light": "resources/light/run-experiment.svg", - "dark": "resources/dark/run-experiment.svg" - } + "icon": "$(debug-rerun)" }, { "title": "%command.queueExperiment%", @@ -303,25 +303,25 @@ "title": "%command.runExperiment%", "command": "dvc.runExperiment", "category": "DVC", - "icon": { - "light": "resources/light/run-experiment.svg", - "dark": "resources/dark/run-experiment.svg" - } + "icon": "$(play)" + }, + { + "title": "%command.resumeCheckpointExperiment%", + "command": "dvc.resumeCheckpointExperiment", + "category": "DVC", + "icon": "$(play)" }, { "title": "%command.runQueuedExperiments%", "command": "dvc.runQueuedExperiments", "category": "DVC", - "icon": { - "dark": "resources/dark/run-all.svg", - "light": "resources/light/run-all.svg" - } + "icon": "$(run-all)" }, { - "title": "%command.runResetExperiment%", - "command": "dvc.runResetExperiment", + "title": "%command.resetAndRunCheckpointExperiment%", + "command": "dvc.resetAndRunCheckpointExperiment", "category": "DVC", - "icon": "$(notebook-revert)" + "icon": "$(debug-rerun)" }, { "title": "%command.selectForCompare%", @@ -359,10 +359,7 @@ "title": "%command.stopRunningExperiment%", "command": "dvc.stopRunningExperiment", "category": "DVC", - "icon": { - "light": "resources/light/stop-experiment.svg", - "dark": "resources/dark/stop-experiment.svg" - } + "icon": "$(debug-stop)" }, { "title": "%command.views.experimentsFilterByTree.removeAllFilters%", @@ -419,19 +416,19 @@ "title": "%command.views.experimentsTree.runExperiment%", "command": "dvc.views.experimentsTree.runExperiment", "category": "DVC", - "icon": { - "light": "resources/light/run-experiment.svg", - "dark": "resources/dark/run-experiment.svg" - } + "icon": "$(play)" }, { - "title": "%command.views.experimentsTree.resetRunExperiment%", - "command": "dvc.views.experimentsTree.resetRunExperiment", + "title": "%command.views.experimentsTree.resumeCheckpointExperiment%", + "command": "dvc.views.experimentsTree.resumeCheckpointExperiment", "category": "DVC", - "icon": { - "light": "resources/light/run-experiment.svg", - "dark": "resources/dark/run-experiment.svg" - } + "icon": "$(play)" + }, + { + "title": "%command.views.experimentsTree.resetAndRunCheckpointExperiment%", + "command": "dvc.views.experimentsTree.resetAndRunCheckpointExperiment", + "category": "DVC", + "icon": "$(debug-rerun)" }, { "title": "%command.views.experimentsTree.selectExperiments%", @@ -618,11 +615,15 @@ }, { "command": "dvc.modifyExperimentParamsAndRun", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" + "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running && !dvc.experiment.checkpoints" + }, + { + "command": "dvc.modifyExperimentParamsAndResume", + "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running && dvc.experiment.checkpoints" }, { "command": "dvc.modifyExperimentParamsResetAndRun", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" + "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running && dvc.experiment.checkpoints" }, { "command": "dvc.queueExperiment", @@ -662,14 +663,22 @@ }, { "command": "dvc.runExperiment", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" + "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running && !dvc.experiment.checkpoints" + }, + { + "command": "dvc.resumeCheckpointExperiment", + "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running && dvc.experiment.checkpoints" + }, + { + "command": "dvc.resetAndRunCheckpointExperiment", + "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running && dvc.experiment.checkpoints" }, { "command": "dvc.runQueuedExperiments", "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" }, { - "command": "dvc.runResetExperiment", + "command": "dvc.resetAndRunCheckpointExperiment", "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" }, { @@ -712,6 +721,14 @@ "command": "dvc.views.experimentsTree.runExperiment", "when": "false" }, + { + "command": "dvc.views.experimentsTree.resumeCheckpointExperiment", + "when": "false" + }, + { + "command": "dvc.views.experimentsTree.resetAndRunCheckpointExperiment", + "when": "false" + }, { "command": "dvc.views.experimentsFilterByTree.removeAllFilters", "when": "false" @@ -824,48 +841,32 @@ "editor/title": [ { "command": "dvc.runExperiment", - "alt": "dvc.runResetExperiment", "group": "navigation@0", - "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available" + "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available && !dvc.experiment.checkpoints" }, { "command": "dvc.stopRunningExperiment", "group": "navigation@0", - "when": "dvc.experiments.webviewActive && dvc.runner.running && dvc.commands.available" + "when": "dvc.runner.running && dvc.commands.available" }, { - "command": "dvc.runQueuedExperiments", + "command": "dvc.resumeCheckpointExperiment", "group": "navigation@1", - "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available" + "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available && dvc.experiment.checkpoints" }, { - "command": "dvc.queueExperiment", + "command": "dvc.resetAndRunCheckpointExperiment", "group": "navigation@2", - "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available" - }, - { - "command": "dvc.runExperiment", - "group": "1_run@0", - "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available" - }, - { - "command": "dvc.stopRunningExperiment", - "group": "1_run@0", - "when": "dvc.experiments.webviewActive && dvc.runner.running && dvc.commands.available" - }, - { - "command": "dvc.queueExperiment", - "group": "1_run@1", - "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available" + "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available && dvc.experiment.checkpoints" }, { "command": "dvc.runQueuedExperiments", - "group": "1_run@2", + "group": "navigation@3", "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available" }, { - "command": "dvc.runResetExperiment", - "group": "1_run@3", + "command": "dvc.queueExperiment", + "group": "navigation@4", "when": "dvc.experiments.webviewActive && !dvc.runner.running && dvc.commands.available" } ], @@ -977,17 +978,22 @@ }, { "command": "dvc.views.experimentsTree.runExperiment", - "group": "1_do@1", - "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.runner.running" + "group": "1_modify@1", + "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.runner.running && !dvc.experiment.checkpoints" }, { - "command": "dvc.views.experimentsTree.resetRunExperiment", - "group": "1_do@2", - "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.runner.running" + "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" + }, + { + "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" }, { "command": "dvc.views.experimentsTree.queueExperiment", - "group": "1_do@3", + "group": "1_modify@3", "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.runner.running" }, { @@ -1034,14 +1040,34 @@ }, { "command": "dvc.runExperiment", - "when": "view == dvc.views.experimentsTree && !dvc.experiments.webviewActive && !dvc.runner.running", + "when": "view == dvc.views.experimentsTree && !dvc.experiments.webviewActive && !dvc.runner.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", "group": "1_run@1" }, + { + "command": "dvc.resetAndRunCheckpointExperiment", + "when": "view == dvc.views.experimentsTree && !dvc.experiments.webviewActive && !dvc.runner.running && dvc.experiment.checkpoints", + "group": "1_run@2" + }, { "command": "dvc.runExperiment", - "when": "view == dvc.views.experimentsTree && dvc.experiments.webviewActive && !dvc.runner.running", + "when": "view == dvc.views.experimentsTree && dvc.experiments.webviewActive && !dvc.runner.running && !dvc.experiment.checkpoints", "group": "navigation@1" }, + { + "command": "dvc.resumeCheckpointExperiment", + "when": "view == dvc.views.experimentsTree && dvc.experiments.webviewActive && !dvc.runner.running && dvc.experiment.checkpoints", + "group": "navigation@1" + }, + { + "command": "dvc.resetAndRunCheckpointExperiment", + "when": "view == dvc.views.experimentsTree && dvc.experiments.webviewActive && !dvc.runner.running && dvc.experiment.checkpoints", + "group": "navigation@2" + }, { "command": "dvc.stopRunningExperiment", "when": "view == dvc.views.experimentsTree && !dvc.experiments.webviewActive && dvc.runner.running", @@ -1052,34 +1078,39 @@ "when": "view == dvc.views.experimentsTree && dvc.experiments.webviewActive && dvc.runner.running", "group": "navigation@1" }, - { - "command": "dvc.runResetExperiment", - "when": "view == dvc.views.experimentsTree && !dvc.runner.running", - "group": "1_run@2" - }, { "command": "dvc.showPlots", "when": "view == dvc.views.experimentsTree && !dvc.plots.webviewActive", - "group": "navigation@2" + "group": "navigation@3" }, { "command": "dvc.views.experimentsTree.selectExperiments", "when": "view == dvc.views.experimentsTree && dvc.plots.webviewActive", - "group": "navigation@2" + "group": "navigation@3" }, { - "command": "dvc.runQueuedExperiments", - "when": "view == dvc.views.experimentsTree && !dvc.runner.running", - "group": "1_run@3" + "command": "dvc.views.experimentsTree.autoApplyFilters", + "when": "view == dvc.views.experimentsTree && !dvc.experiments.filter.selected", + "group": "navigation@4" + }, + { + "command": "dvc.views.experimentsTree.disableAutoApplyFilters", + "when": "view == dvc.views.experimentsTree && dvc.experiments.filter.selected", + "group": "navigation@4" }, { "command": "dvc.modifyExperimentParamsAndRun", - "when": "view == dvc.views.experimentsTree && !dvc.runner.running", + "when": "view == dvc.views.experimentsTree && !dvc.runner.running && !dvc.experiment.checkpoints", + "group": "2_modify@1" + }, + { + "command": "dvc.modifyExperimentParamsAndResume", + "when": "view == dvc.views.experimentsTree && !dvc.runner.running && dvc.experiment.checkpoints", "group": "2_modify@1" }, { "command": "dvc.modifyExperimentParamsResetAndRun", - "when": "view == dvc.views.experimentsTree && !dvc.runner.running", + "when": "view == dvc.views.experimentsTree && !dvc.runner.running && dvc.experiment.checkpoints", "group": "2_modify@2" }, { @@ -1093,14 +1124,9 @@ "group": "3_queue@1" }, { - "command": "dvc.views.experimentsTree.autoApplyFilters", - "when": "view == dvc.views.experimentsTree && !dvc.experiments.filter.selected", - "group": "navigation@4" - }, - { - "command": "dvc.views.experimentsTree.disableAutoApplyFilters", - "when": "view == dvc.views.experimentsTree && dvc.experiments.filter.selected", - "group": "navigation@4" + "command": "dvc.runQueuedExperiments", + "when": "view == dvc.views.experimentsTree && !dvc.runner.running", + "group": "3_queue@2" }, { "command": "dvc.addExperimentsTableSort", diff --git a/extension/package.nls.json b/extension/package.nls.json index 55004e749b..9427b5a354 100644 --- a/extension/package.nls.json +++ b/extension/package.nls.json @@ -30,7 +30,8 @@ "command.pushTarget": "Push", "command.modifyExperimentParamsAndQueue": "Modify Experiment Param(s) and Queue", "command.modifyExperimentParamsAndRun": "Modify Experiment Param(s) and Run", - "command.modifyExperimentParamsResetAndRun": "Modify Experiment Param(s) Reset and Run", + "command.modifyExperimentParamsAndResume": "Modify Experiment Param(s) and Resume", + "command.modifyExperimentParamsResetAndRun": "Modify Experiment Param(s), Reset and Run", "command.queueExperiment": "Queue Experiment", "command.removeExperiment": "Remove Experiment", "command.removeExperimentQueue": "Remove All Queued Experiments", @@ -41,8 +42,9 @@ "command.renameTarget": "Rename", "command.resetWorkspace": "Reset the Workspace", "command.runExperiment": "Run Experiment", + "command.resumeCheckpointExperiment": "Resume Checkpoint Experiment", "command.runQueuedExperiments": "Run All Queued Experiments", - "command.runResetExperiment": "Reset and Run Experiment", + "command.resetAndRunCheckpointExperiment": "Reset and Run Checkpoint Experiment", "command.selectForCompare": "Select for Compare", "command.setupWorkspace": "Setup The Workspace", "command.showCommands": "Show Commands", @@ -61,7 +63,8 @@ "command.views.experimentsTree.queueExperiment": "Modify Param(s) and Queue", "command.views.experimentsTree.removeExperiment": "Remove", "command.views.experimentsTree.runExperiment": "Modify Param(s) and Run", - "command.views.experimentsTree.resetRunExperiment": "Modify Param(s) Reset and Run", + "command.views.experimentsTree.resumeCheckpointExperiment": "Modify Param(s) and Resume", + "command.views.experimentsTree.resetAndRunCheckpointExperiment": "Modify Param(s), Reset and Run", "command.views.experimentsTree.selectExperiments": "Select Experiments to Display in Plots", "command.views.plotsPathsTree.selectPlots": "Select Plots to Display", "config.doNotRecommendRedHatExtension.description": "Do not prompt to install the Red Hat YAML extension to assist with DVC YAML schema validation.", diff --git a/extension/resources/dark/run-all.svg b/extension/resources/dark/run-all.svg deleted file mode 100644 index 8ae6a17390..0000000000 --- a/extension/resources/dark/run-all.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/extension/resources/dark/run-experiment.svg b/extension/resources/dark/run-experiment.svg deleted file mode 100644 index dff59d48a9..0000000000 --- a/extension/resources/dark/run-experiment.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/extension/resources/dark/stop-experiment.svg b/extension/resources/dark/stop-experiment.svg deleted file mode 100644 index 44d4e6b7cb..0000000000 --- a/extension/resources/dark/stop-experiment.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/extension/resources/light/run-all.svg b/extension/resources/light/run-all.svg deleted file mode 100644 index b7d4f18f11..0000000000 --- a/extension/resources/light/run-all.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/extension/resources/light/run-experiment.svg b/extension/resources/light/run-experiment.svg deleted file mode 100644 index 7c3452fc20..0000000000 --- a/extension/resources/light/run-experiment.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/extension/resources/light/stop-experiment.svg b/extension/resources/light/stop-experiment.svg deleted file mode 100644 index 1fe0b8bd51..0000000000 --- a/extension/resources/light/stop-experiment.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/extension/src/cli/runner.ts b/extension/src/cli/runner.ts index 34ae979935..1922257664 100644 --- a/extension/src/cli/runner.ts +++ b/extension/src/cli/runner.ts @@ -18,9 +18,9 @@ import { Toast } from '../vscode/toast' import { Disposable } from '../class/dispose' export const autoRegisteredCommands = { + EXPERIMENT_RESET_AND_RUN: 'runExperimentReset', EXPERIMENT_RUN: 'runExperiment', EXPERIMENT_RUN_QUEUED: 'runExperimentQueue', - EXPERIMENT_RUN_RESET: 'runExperimentReset', IS_EXPERIMENT_RUNNING: 'isExperimentRunning' } as const diff --git a/extension/src/commands/external.ts b/extension/src/commands/external.ts index 884f4c4658..966319256f 100644 --- a/extension/src/commands/external.ts +++ b/extension/src/commands/external.ts @@ -5,9 +5,10 @@ export enum RegisteredCliCommands { EXPERIMENT_REMOVE = 'dvc.removeExperiment', EXPERIMENT_REMOVE_QUEUE = 'dvc.removeExperimentQueue', EXPERIMENT_REMOVE_QUEUED = 'dvc.removeQueuedExperiment', + EXPERIMENT_RESUME = 'dvc.resumeCheckpointExperiment', EXPERIMENT_RUN = 'dvc.runExperiment', EXPERIMENT_RUN_QUEUED = 'dvc.runQueuedExperiments', - EXPERIMENT_RUN_RESET = 'dvc.runResetExperiment', + EXPERIMENT_RESET_AND_RUN = 'dvc.resetAndRunCheckpointExperiment', QUEUE_EXPERIMENT = 'dvc.queueExperiment', ADD_TARGET = 'dvc.addTarget', @@ -36,8 +37,9 @@ export enum RegisteredCommands { EXPERIMENT_TREE_BRANCH = 'dvc.views.experimentsTree.branchExperiment', EXPERIMENT_TREE_QUEUE = 'dvc.views.experimentsTree.queueExperiment', EXPERIMENT_TREE_REMOVE = 'dvc.views.experimentsTree.removeExperiment', + EXPERIMENT_TREE_RESUME = 'dvc.views.experimentsTree.resumeCheckpointExperiment', EXPERIMENT_TREE_RUN = 'dvc.views.experimentsTree.runExperiment', - EXPERIMENT_TREE_RUN_RESET = 'dvc.views.experimentsTree.resetRunExperiment', + EXPERIMENT_TREE_RESET_AND_RUN = 'dvc.views.experimentsTree.resetAndRunCheckpointExperiment', EXPERIMENT_SELECT = 'dvc.views.experimentsTree.selectExperiments', EXPERIMENT_SHOW = 'dvc.showExperiments', EXPERIMENT_SORT_ADD = 'dvc.addExperimentsTableSort', @@ -46,6 +48,7 @@ export enum RegisteredCommands { EXPERIMENT_SORTS_REMOVE_ALL = 'dvc.views.experimentsSortByTree.removeAllSorts', EXPERIMENT_TOGGLE = 'dvc.views.experimentsTree.toggleStatus', MODIFY_EXPERIMENT_PARAMS_AND_QUEUE = 'dvc.modifyExperimentParamsAndQueue', + MODIFY_EXPERIMENT_PARAMS_AND_RESUME = 'dvc.modifyExperimentParamsAndResume', MODIFY_EXPERIMENT_PARAMS_AND_RUN = 'dvc.modifyExperimentParamsAndRun', MODIFY_EXPERIMENT_PARAMS_RESET_AND_RUN = 'dvc.modifyExperimentParamsResetAndRun', STOP_EXPERIMENT = 'dvc.stopRunningExperiment', diff --git a/extension/src/experiments/commands/register.ts b/extension/src/experiments/commands/register.ts index 5c9ccdd1ef..0d7423b2db 100644 --- a/extension/src/experiments/commands/register.ts +++ b/extension/src/experiments/commands/register.ts @@ -29,14 +29,19 @@ const registerExperimentCwdCommands = ( ) ) + const modifyExperimentParamsAndRun = () => + experiments.pauseUpdatesThenRun(() => + experiments.modifyExperimentParamsAndRun(AvailableCommands.EXPERIMENT_RUN) + ) + + internalCommands.registerExternalCommand( + RegisteredCommands.MODIFY_EXPERIMENT_PARAMS_AND_RESUME, + modifyExperimentParamsAndRun + ) + internalCommands.registerExternalCommand( RegisteredCommands.MODIFY_EXPERIMENT_PARAMS_AND_RUN, - () => - experiments.pauseUpdatesThenRun(() => - experiments.modifyExperimentParamsAndRun( - AvailableCommands.EXPERIMENT_RUN - ) - ) + modifyExperimentParamsAndRun ) internalCommands.registerExternalCommand( @@ -44,7 +49,7 @@ const registerExperimentCwdCommands = ( () => experiments.pauseUpdatesThenRun(() => experiments.modifyExperimentParamsAndRun( - AvailableCommands.EXPERIMENT_RUN_RESET + AvailableCommands.EXPERIMENT_RESET_AND_RUN ) ) ) @@ -138,8 +143,13 @@ const registerExperimentRunCommands = ( ) internalCommands.registerExternalCliCommand( - RegisteredCliCommands.EXPERIMENT_RUN_RESET, - () => experiments.getCwdThenRun(AvailableCommands.EXPERIMENT_RUN_RESET) + RegisteredCliCommands.EXPERIMENT_RESUME, + () => experiments.getCwdThenRun(AvailableCommands.EXPERIMENT_RUN) + ) + + internalCommands.registerExternalCliCommand( + RegisteredCliCommands.EXPERIMENT_RESET_AND_RUN, + () => experiments.getCwdThenRun(AvailableCommands.EXPERIMENT_RESET_AND_RUN) ) internalCommands.registerExternalCliCommand( diff --git a/extension/src/experiments/index.ts b/extension/src/experiments/index.ts index f862d5cb0f..ca3c7e5657 100644 --- a/extension/src/experiments/index.ts +++ b/extension/src/experiments/index.ts @@ -417,7 +417,7 @@ export class Experiments extends BaseRepository { ) case MessageFromWebviewType.VARY_EXPERIMENT_PARAMS_RESET_AND_RUN: return this.modifyExperimentParamsAndRun( - AvailableCommands.EXPERIMENT_RUN_RESET, + AvailableCommands.EXPERIMENT_RESET_AND_RUN, message.payload ) diff --git a/extension/src/experiments/model/tree.ts b/extension/src/experiments/model/tree.ts index 6734066d57..a2ab21597a 100644 --- a/extension/src/experiments/model/tree.ts +++ b/extension/src/experiments/model/tree.ts @@ -154,21 +154,28 @@ export class ExperimentsTree ) ) + const modifyExperimentParamsAndRun = ({ dvcRoot, id }: ExperimentItem) => + this.experiments.modifyExperimentParamsAndRun( + AvailableCommands.EXPERIMENT_RUN, + dvcRoot, + id + ) + internalCommands.registerExternalCommand( RegisteredCommands.EXPERIMENT_TREE_RUN, - ({ dvcRoot, id }: ExperimentItem) => - this.experiments.modifyExperimentParamsAndRun( - AvailableCommands.EXPERIMENT_RUN, - dvcRoot, - id - ) + modifyExperimentParamsAndRun + ) + + internalCommands.registerExternalCommand( + RegisteredCommands.EXPERIMENT_TREE_RESUME, + modifyExperimentParamsAndRun ) internalCommands.registerExternalCommand( - RegisteredCommands.EXPERIMENT_TREE_RUN_RESET, + RegisteredCommands.EXPERIMENT_TREE_RESET_AND_RUN, ({ dvcRoot, id }: ExperimentItem) => this.experiments.modifyExperimentParamsAndRun( - AvailableCommands.EXPERIMENT_RUN_RESET, + AvailableCommands.EXPERIMENT_RESET_AND_RUN, dvcRoot, id ) diff --git a/extension/src/telemetry/constants.ts b/extension/src/telemetry/constants.ts index 3a34920584..9cf9eb8069 100644 --- a/extension/src/telemetry/constants.ts +++ b/extension/src/telemetry/constants.ts @@ -107,9 +107,10 @@ export interface IEventNamePropertyMapping { [EventName.EXPERIMENT_REMOVE]: undefined [EventName.EXPERIMENT_REMOVE_QUEUE]: undefined [EventName.EXPERIMENT_REMOVE_QUEUED]: undefined + [EventName.EXPERIMENT_RESUME]: undefined [EventName.EXPERIMENT_RUN]: undefined [EventName.EXPERIMENT_RUN_QUEUED]: undefined - [EventName.EXPERIMENT_RUN_RESET]: undefined + [EventName.EXPERIMENT_RESET_AND_RUN]: undefined [EventName.EXPERIMENT_SELECT]: undefined [EventName.EXPERIMENT_SHOW]: undefined [EventName.EXPERIMENT_SORT_ADD]: undefined @@ -120,11 +121,13 @@ export interface IEventNamePropertyMapping { [EventName.EXPERIMENT_TREE_BRANCH]: undefined [EventName.EXPERIMENT_TREE_QUEUE]: undefined [EventName.EXPERIMENT_TREE_REMOVE]: undefined + [EventName.EXPERIMENT_TREE_RESUME]: undefined [EventName.EXPERIMENT_TREE_RUN]: undefined - [EventName.EXPERIMENT_TREE_RUN_RESET]: undefined + [EventName.EXPERIMENT_TREE_RESET_AND_RUN]: undefined [EventName.EXPERIMENT_TOGGLE]: undefined [EventName.QUEUE_EXPERIMENT]: undefined [EventName.MODIFY_EXPERIMENT_PARAMS_AND_QUEUE]: undefined + [EventName.MODIFY_EXPERIMENT_PARAMS_AND_RESUME]: undefined [EventName.MODIFY_EXPERIMENT_PARAMS_AND_RUN]: undefined [EventName.MODIFY_EXPERIMENT_PARAMS_RESET_AND_RUN]: undefined [EventName.STOP_EXPERIMENT]: { stopped: boolean; wasRunning: boolean } diff --git a/extension/src/test/suite/experiments/index.test.ts b/extension/src/test/suite/experiments/index.test.ts index 8afcf2d6f4..2e2acbfe84 100644 --- a/extension/src/test/suite/experiments/index.test.ts +++ b/extension/src/test/suite/experiments/index.test.ts @@ -480,7 +480,7 @@ suite('Experiments Test Suite', () => { await tableChangePromise expect(mockExecuteCommand).to.be.calledOnce expect(mockExecuteCommand).to.be.calledWithExactly( - AvailableCommands.EXPERIMENT_RUN_RESET, + AvailableCommands.EXPERIMENT_RESET_AND_RUN, dvcDemoPath, ...mockModifiedParams ) diff --git a/extension/src/test/suite/experiments/model/tree.test.ts b/extension/src/test/suite/experiments/model/tree.test.ts index 5c61595aef..82bc3e240d 100644 --- a/extension/src/test/suite/experiments/model/tree.test.ts +++ b/extension/src/test/suite/experiments/model/tree.test.ts @@ -624,7 +624,7 @@ suite('Experiments Tree Test Suite', () => { .resolves('0.82') await commands.executeCommand( - RegisteredCommands.EXPERIMENT_TREE_RUN_RESET, + RegisteredCommands.EXPERIMENT_TREE_RESET_AND_RUN, { dvcRoot: dvcDemoPath, id: baseExperimentId diff --git a/extension/src/test/suite/experiments/workspace.test.ts b/extension/src/test/suite/experiments/workspace.test.ts index be2b6bb88c..40aa30ba63 100644 --- a/extension/src/test/suite/experiments/workspace.test.ts +++ b/extension/src/test/suite/experiments/workspace.test.ts @@ -154,6 +154,70 @@ suite('Workspace Experiments Test Suite', () => { }) }) + describe('dvc.modifyExperimentParamsAndResume', () => { + it('should be able to resume a checkpoint experiment using an existing one as a base', async () => { + const { experiments } = buildExperiments(disposable) + + const mockExperimentRun = stub( + CliRunner.prototype, + 'runExperiment' + ).resolves(undefined) + + stub( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (WorkspaceExperiments as any).prototype, + 'getOnlyOrPickProject' + ).returns(dvcDemoPath) + + stub(WorkspaceExperiments.prototype, 'getRepository').returns(experiments) + + const mockShowQuickPick = stub(window, 'showQuickPick') as SinonStub< + [items: readonly QuickPickItem[], options: QuickPickOptionsWithTitle], + Thenable< + QuickPickItem[] | QuickPickItemWithValue<{ id: string }> | undefined + > + > + mockShowQuickPick + .onFirstCall() + .resolves({ value: { id: 'workspace' } } as QuickPickItemWithValue<{ + id: string + }>) + .onSecondCall() + .resolves([ + { + label: 'params.yaml:dropout', + value: { path: 'params.yaml:dropout', value: 0.1 } + }, + { + label: 'params.yaml:process.threshold', + value: { path: 'params.yaml:process.threshold', value: 0.15 } + } + ] as QuickPickItemWithValue[]) + + const dropout = '0.222222' + const threshold = '0.1665' + + stub(window, 'showInputBox') + .onFirstCall() + .resolves(dropout) + .onSecondCall() + .resolves(threshold) + + await commands.executeCommand( + RegisteredCommands.MODIFY_EXPERIMENT_PARAMS_AND_RESUME + ) + + expect(mockExperimentRun).to.be.calledOnce + expect(mockExperimentRun).to.be.calledWith( + dvcDemoPath, + '-S', + `params.yaml:dropout=${dropout}`, + '-S', + `params.yaml:process.threshold=${threshold}` + ) + }) + }) + describe('dvc.modifyExperimentParamsAndRun', () => { it('should be able to run an experiment using an existing one as a base', async () => { const { experiments } = buildExperiments(disposable) @@ -316,7 +380,7 @@ suite('Workspace Experiments Test Suite', () => { }) }) - describe('dvc.runResetExperiment', () => { + describe('dvc.resetAndRunCheckpointExperiment', () => { it('should be able to reset existing checkpoints and restart the experiment', async () => { const mockRunExperimentReset = stub( CliRunner.prototype, @@ -329,7 +393,9 @@ suite('Workspace Experiments Test Suite', () => { 'getOnlyOrPickProject' ).returns(dvcDemoPath) - await commands.executeCommand(RegisteredCliCommands.EXPERIMENT_RUN_RESET) + await commands.executeCommand( + RegisteredCliCommands.EXPERIMENT_RESET_AND_RUN + ) expect(mockRunExperimentReset).to.be.calledOnce expect(mockRunExperimentReset).to.be.calledWith(dvcDemoPath) From 2f35e448afed8352fc01ce46d020b1008fb1383c Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Thu, 19 May 2022 14:48:26 +1000 Subject: [PATCH 3/5] add further intergration tests --- .../test/suite/experiments/model/tree.test.ts | 63 ++++++++++++++++++- .../test/suite/experiments/workspace.test.ts | 20 ++++++ 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/extension/src/test/suite/experiments/model/tree.test.ts b/extension/src/test/suite/experiments/model/tree.test.ts index 82bc3e240d..1dc1533077 100644 --- a/extension/src/test/suite/experiments/model/tree.test.ts +++ b/extension/src/test/suite/experiments/model/tree.test.ts @@ -580,7 +580,68 @@ suite('Experiments Tree Test Suite', () => { ) }) - it('should be able to reset and run a new experiment from an existing one with dvc.views.experimentsTree.resetRunExperiment', async () => { + it('should be able to run a new experiment from an existing one with dvc.views.experimentsTree.runExperiment', async () => { + const baseExperimentId = 'workspace' + + const { cliRunner, experiments, experimentsModel } = + buildExperiments(disposable) + + await experiments.isReady() + + const mockRunExperiment = stub(cliRunner, 'runExperiment').resolves( + undefined + ) + + const mockGetOnlyOrPickProject = stub( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (WorkspaceExperiments as any).prototype, + 'getOnlyOrPickProject' + ) + stub(WorkspaceExperiments.prototype, 'getRepository').returns(experiments) + + const getParamsSpy = spy(experimentsModel, 'getExperimentParams') + + const mockShowQuickPick = stub(window, 'showQuickPick') as SinonStub< + [items: readonly QuickPickItem[], options: QuickPickOptionsWithTitle], + Thenable | undefined> + > + mockShowQuickPick.resolves([ + { + label: 'params.yaml:dropout', + value: { path: 'params.yaml:dropout', value: 0.16 } + }, + { + label: 'params.yaml:process.threshold', + value: { path: 'params.yaml:process.threshold', value: 0.1 } + } + ] as QuickPickItemWithValue[]) + + stub(window, 'showInputBox') + .onFirstCall() + .resolves('0.15') + .onSecondCall() + .resolves('0.82') + + await commands.executeCommand(RegisteredCommands.EXPERIMENT_TREE_RUN, { + dvcRoot: dvcDemoPath, + id: baseExperimentId + }) + + expect(mockGetOnlyOrPickProject).not.to.be.called + expect(getParamsSpy).to.be.calledOnce + expect(getParamsSpy).to.be.calledWithExactly(baseExperimentId) + expect(mockShowQuickPick).to.be.calledOnce + expect(mockRunExperiment).to.be.calledOnce + expect(mockRunExperiment).to.be.calledWith( + dvcDemoPath, + '-S', + 'params.yaml:dropout=0.15', + '-S', + 'params.yaml:process.threshold=0.82' + ) + }) + + it('should be able to reset and run a new checkpoint experiment from an existing one with dvc.views.experimentsTree.resetRunExperiment', async () => { const baseExperimentId = 'workspace' const { cliRunner, experiments, experimentsModel } = diff --git a/extension/src/test/suite/experiments/workspace.test.ts b/extension/src/test/suite/experiments/workspace.test.ts index 40aa30ba63..25cd5fca09 100644 --- a/extension/src/test/suite/experiments/workspace.test.ts +++ b/extension/src/test/suite/experiments/workspace.test.ts @@ -380,6 +380,26 @@ suite('Workspace Experiments Test Suite', () => { }) }) + describe('dvc.resumeCheckpointExperiment', () => { + it('should be able to run an experiment', async () => { + const mockRunExperiment = stub( + CliRunner.prototype, + 'runExperiment' + ).resolves(undefined) + + stub( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (WorkspaceExperiments as any).prototype, + 'getOnlyOrPickProject' + ).returns(dvcDemoPath) + + await commands.executeCommand(RegisteredCliCommands.EXPERIMENT_RESUME) + + expect(mockRunExperiment).to.be.calledOnce + expect(mockRunExperiment).to.be.calledWith(dvcDemoPath) + }) + }) + describe('dvc.resetAndRunCheckpointExperiment', () => { it('should be able to reset existing checkpoints and restart the experiment', async () => { const mockRunExperimentReset = stub( From c1a1651ef70c828295c579e5581dbd9aa7619b73 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Thu, 19 May 2022 15:13:59 +1000 Subject: [PATCH 4/5] remove extra command from palette --- extension/package.json | 4 ---- 1 file changed, 4 deletions(-) diff --git a/extension/package.json b/extension/package.json index 3f7e712f67..069946ecfd 100644 --- a/extension/package.json +++ b/extension/package.json @@ -677,10 +677,6 @@ "command": "dvc.runQueuedExperiments", "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" }, - { - "command": "dvc.resetAndRunCheckpointExperiment", - "when": "dvc.commands.available && dvc.project.available && !dvc.runner.running" - }, { "command": "dvc.selectForCompare", "when": "false" From 03ed32042e6993c688a2d1d7b983cce8abe3aa95 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Fri, 20 May 2022 08:11:26 +1000 Subject: [PATCH 5/5] increase timeout of webview tests --- webview/jest.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webview/jest.config.js b/webview/jest.config.js index 8f9af31e96..4c8bf2774f 100644 --- a/webview/jest.config.js +++ b/webview/jest.config.js @@ -20,5 +20,5 @@ module.exports = { preset: 'ts-jest', setupFiles: ['jest-canvas-mock', '/setup-tests.js'], testEnvironment: 'node', - testTimeout: 10000 + testTimeout: 20000 }