From 9f572a1591d6f072bad55beedcbee11c53f0387b Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Fri, 19 Aug 2022 16:01:16 +1000 Subject: [PATCH 1/3] share experiment from palette --- extension/package.json | 10 ++++++++ extension/package.nls.json | 1 + extension/src/commands/external.ts | 1 + .../src/experiments/commands/register.ts | 23 +++++++++++++++++++ extension/src/telemetry/constants.ts | 1 + 5 files changed, 36 insertions(+) diff --git a/extension/package.json b/extension/package.json index 438488a072..ae85792c62 100644 --- a/extension/package.json +++ b/extension/package.json @@ -357,6 +357,12 @@ "command": "dvc.setupWorkspace", "category": "DVC" }, + { + "title": "%command.shareExperimentAsBranch%", + "command": "dvc.shareExperimentAsBranch", + "category": "DVC", + "icon": "$(repo-push)" + }, { "title": "%command.showCommands", "command": "dvc.showCommands", @@ -741,6 +747,10 @@ "command": "dvc.showCommands", "when": "false" }, + { + "command": "dvc.shareExperimentAsBranch", + "when": "dvc.commands.available && dvc.project.available && !dvc.experiment.running" + }, { "command": "dvc.showExperiments", "when": "dvc.commands.available && dvc.project.available" diff --git a/extension/package.nls.json b/extension/package.nls.json index 0de3f478e7..67fd5d9f29 100644 --- a/extension/package.nls.json +++ b/extension/package.nls.json @@ -48,6 +48,7 @@ "command.resetAndRunCheckpointExperiment": "Reset and Run Experiment", "command.selectForCompare": "Select for Compare", "command.setupWorkspace": "Setup The Workspace", + "command.shareExperimentAsBranch": "Share Experiment as Branch", "command.showCommands": "Show Commands", "command.showExperiments": "Show Experiments", "command.showOutput": "Show DVC Output", diff --git a/extension/src/commands/external.ts b/extension/src/commands/external.ts index 1708b11e5d..7bb72d236c 100644 --- a/extension/src/commands/external.ts +++ b/extension/src/commands/external.ts @@ -9,6 +9,7 @@ export enum RegisteredCliCommands { EXPERIMENT_RUN = 'dvc.runExperiment', EXPERIMENT_RUN_QUEUED = 'dvc.startExperimentsQueue', EXPERIMENT_RESET_AND_RUN = 'dvc.resetAndRunCheckpointExperiment', + EXPERIMENT_SHARE_AS_BRANCH = 'dvc.shareExperimentAsBranch', QUEUE_EXPERIMENT = 'dvc.queueExperiment', EXPERIMENT_VIEW_APPLY = 'dvc.views.experiments.applyExperiment', diff --git a/extension/src/experiments/commands/register.ts b/extension/src/experiments/commands/register.ts index 4668560379..c5de721cf8 100644 --- a/extension/src/experiments/commands/register.ts +++ b/extension/src/experiments/commands/register.ts @@ -166,6 +166,29 @@ const registerExperimentInputCommands = ( ) ) + internalCommands.registerExternalCliCommand( + RegisteredCliCommands.EXPERIMENT_SHARE_AS_BRANCH, + () => + experiments.getCwdExpNameAndInputThenRun( + async (cwd: string, name: string, input: string) => { + await experiments.runCommand( + AvailableCommands.EXPERIMENT_BRANCH, + cwd, + name, + input + ) + await experiments.runCommand( + AvailableCommands.EXPERIMENT_APPLY, + cwd, + name + ) + await experiments.runCommand(AvailableCommands.PUSH, cwd) + return Toast.showOutput(gitPushBranch(cwd, input)) + }, + Title.ENTER_BRANCH_NAME + ) + ) + internalCommands.registerExternalCliCommand( RegisteredCliCommands.EXPERIMENT_VIEW_BRANCH, ({ dvcRoot, id }: ExperimentDetails) => diff --git a/extension/src/telemetry/constants.ts b/extension/src/telemetry/constants.ts index ec6abc47fa..9e898e742a 100644 --- a/extension/src/telemetry/constants.ts +++ b/extension/src/telemetry/constants.ts @@ -126,6 +126,7 @@ export interface IEventNamePropertyMapping { [EventName.EXPERIMENT_RUN_QUEUED]: undefined [EventName.EXPERIMENT_RESET_AND_RUN]: undefined [EventName.EXPERIMENT_SELECT]: undefined + [EventName.EXPERIMENT_SHARE_AS_BRANCH]: undefined [EventName.EXPERIMENT_SHOW]: undefined [EventName.EXPERIMENT_SORT_ADD]: undefined [EventName.EXPERIMENT_SORT_ADD_STARRED]: undefined From 6d0cbb2a85cd7e88c6ee1eb2bc917352b3c4c555 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Fri, 19 Aug 2022 16:27:59 +1000 Subject: [PATCH 2/3] reduce duplication --- extension/src/experiments/commands/index.ts | 27 ++++++++++ .../src/experiments/commands/register.ts | 54 +++---------------- extension/src/experiments/workspace.ts | 2 +- 3 files changed, 36 insertions(+), 47 deletions(-) create mode 100644 extension/src/experiments/commands/index.ts diff --git a/extension/src/experiments/commands/index.ts b/extension/src/experiments/commands/index.ts new file mode 100644 index 0000000000..cd4b60bf1a --- /dev/null +++ b/extension/src/experiments/commands/index.ts @@ -0,0 +1,27 @@ +import { AvailableCommands } from '../../commands/internal' +import { gitPushBranch } from '../../git' +import { Toast } from '../../vscode/toast' +import { WorkspaceExperiments } from '../workspace' + +export const getBranchExperimentCommand = + (experiments: WorkspaceExperiments) => + (cwd: string, name: string, input: string) => + experiments.runCommand( + AvailableCommands.EXPERIMENT_BRANCH, + cwd, + name, + input + ) + +export const getShareExperimentAsBranchCommand = + (experiments: WorkspaceExperiments) => + async (cwd: string, name: string, input: string) => { + const branchCommand = getBranchExperimentCommand(experiments) + await branchCommand(cwd, name, input) + + await experiments.runCommand(AvailableCommands.EXPERIMENT_APPLY, cwd, name) + + await experiments.runCommand(AvailableCommands.PUSH, cwd) + + return Toast.showOutput(gitPushBranch(cwd, input)) + } diff --git a/extension/src/experiments/commands/register.ts b/extension/src/experiments/commands/register.ts index c5de721cf8..75cb7b3624 100644 --- a/extension/src/experiments/commands/register.ts +++ b/extension/src/experiments/commands/register.ts @@ -1,3 +1,7 @@ +import { + getBranchExperimentCommand, + getShareExperimentAsBranchCommand +} from '.' import { pickGarbageCollectionFlags } from '../quickPick' import { WorkspaceExperiments } from '../workspace' import { AvailableCommands, InternalCommands } from '../../commands/internal' @@ -6,9 +10,6 @@ import { RegisteredCommands } from '../../commands/external' import { Title } from '../../vscode/title' -import { gitPushBranch } from '../../git' -import { Toast } from '../../vscode/toast' -import { Args } from '../../cli/constants' type ExperimentDetails = { dvcRoot: string; id: string } @@ -156,12 +157,7 @@ const registerExperimentInputCommands = ( RegisteredCliCommands.EXPERIMENT_BRANCH, () => experiments.getCwdExpNameAndInputThenRun( - (cwd, ...args: Args) => - experiments.runCommand( - AvailableCommands.EXPERIMENT_BRANCH, - cwd, - ...args - ), + getBranchExperimentCommand(experiments), Title.ENTER_BRANCH_NAME ) ) @@ -170,21 +166,7 @@ const registerExperimentInputCommands = ( RegisteredCliCommands.EXPERIMENT_SHARE_AS_BRANCH, () => experiments.getCwdExpNameAndInputThenRun( - async (cwd: string, name: string, input: string) => { - await experiments.runCommand( - AvailableCommands.EXPERIMENT_BRANCH, - cwd, - name, - input - ) - await experiments.runCommand( - AvailableCommands.EXPERIMENT_APPLY, - cwd, - name - ) - await experiments.runCommand(AvailableCommands.PUSH, cwd) - return Toast.showOutput(gitPushBranch(cwd, input)) - }, + getShareExperimentAsBranchCommand(experiments), Title.ENTER_BRANCH_NAME ) ) @@ -193,13 +175,7 @@ const registerExperimentInputCommands = ( RegisteredCliCommands.EXPERIMENT_VIEW_BRANCH, ({ dvcRoot, id }: ExperimentDetails) => experiments.getExpNameAndInputThenRun( - (name: string, input: string) => - experiments.runCommand( - AvailableCommands.EXPERIMENT_BRANCH, - dvcRoot, - name, - input - ), + getBranchExperimentCommand(experiments), Title.ENTER_BRANCH_NAME, dvcRoot, id @@ -210,21 +186,7 @@ const registerExperimentInputCommands = ( RegisteredCliCommands.EXPERIMENT_VIEW_SHARE_AS_BRANCH, ({ dvcRoot, id }: ExperimentDetails) => experiments.getExpNameAndInputThenRun( - async (name: string, input: string) => { - await experiments.runCommand( - AvailableCommands.EXPERIMENT_BRANCH, - dvcRoot, - name, - input - ) - await experiments.runCommand( - AvailableCommands.EXPERIMENT_APPLY, - dvcRoot, - name - ) - await experiments.runCommand(AvailableCommands.PUSH, dvcRoot) - return Toast.showOutput(gitPushBranch(dvcRoot, input)) - }, + getShareExperimentAsBranchCommand(experiments), Title.ENTER_BRANCH_NAME, dvcRoot, id diff --git a/extension/src/experiments/workspace.ts b/extension/src/experiments/workspace.ts index 071161d5cd..8b609d3f48 100644 --- a/extension/src/experiments/workspace.ts +++ b/extension/src/experiments/workspace.ts @@ -251,7 +251,7 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews< return } - return this.getInputAndRun(runCommand, title, name) + return this.getInputAndRun(runCommand, title, cwd, name) } public getExpNameThenRun(commandId: CommandId, cwd: string, id: string) { From b1056724e3c09f0ed0b11fb4af015f7c6f9da4ce Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Fri, 19 Aug 2022 17:28:05 +1000 Subject: [PATCH 3/3] add integration tests for branch and share as branch --- .../src/experiments/commands/register.ts | 18 +-- .../test/suite/experiments/workspace.test.ts | 110 +++++++++++++++++- 2 files changed, 118 insertions(+), 10 deletions(-) diff --git a/extension/src/experiments/commands/register.ts b/extension/src/experiments/commands/register.ts index 75cb7b3624..3ae4330cfe 100644 --- a/extension/src/experiments/commands/register.ts +++ b/extension/src/experiments/commands/register.ts @@ -162,15 +162,6 @@ const registerExperimentInputCommands = ( ) ) - internalCommands.registerExternalCliCommand( - RegisteredCliCommands.EXPERIMENT_SHARE_AS_BRANCH, - () => - experiments.getCwdExpNameAndInputThenRun( - getShareExperimentAsBranchCommand(experiments), - Title.ENTER_BRANCH_NAME - ) - ) - internalCommands.registerExternalCliCommand( RegisteredCliCommands.EXPERIMENT_VIEW_BRANCH, ({ dvcRoot, id }: ExperimentDetails) => @@ -182,6 +173,15 @@ const registerExperimentInputCommands = ( ) ) + internalCommands.registerExternalCliCommand( + RegisteredCliCommands.EXPERIMENT_SHARE_AS_BRANCH, + () => + experiments.getCwdExpNameAndInputThenRun( + getShareExperimentAsBranchCommand(experiments), + Title.ENTER_BRANCH_NAME + ) + ) + internalCommands.registerExternalCliCommand( RegisteredCliCommands.EXPERIMENT_VIEW_SHARE_AS_BRANCH, ({ dvcRoot, id }: ExperimentDetails) => diff --git a/extension/src/test/suite/experiments/workspace.test.ts b/extension/src/test/suite/experiments/workspace.test.ts index 688cf91105..c3f2fdb7f8 100644 --- a/extension/src/test/suite/experiments/workspace.test.ts +++ b/extension/src/test/suite/experiments/workspace.test.ts @@ -12,7 +12,7 @@ import { WorkspaceExperiments } from '../../../experiments/workspace' import { Experiments } from '../../../experiments' import * as QuickPick from '../../../vscode/quickPick' import { CliExecutor } from '../../../cli/executor' -import { closeAllEditors, mockDuration } from '../util' +import { closeAllEditors, getInputBoxEvent, mockDuration } from '../util' import { dvcDemoPath } from '../../util' import { RegisteredCliCommands } from '../../../commands/external' import * as Telemetry from '../../../telemetry' @@ -26,6 +26,7 @@ import { WEBVIEW_TEST_TIMEOUT } from '../timeouts' import { Title } from '../../../vscode/title' import { join } from '../../util/path' import { AvailableCommands } from '../../../commands/internal' +import * as Git from '../../../git' suite('Workspace Experiments Test Suite', () => { const disposable = Disposable.fn() @@ -564,6 +565,113 @@ suite('Workspace Experiments Test Suite', () => { }) }) + describe('dvc.branchExperiment', () => { + it('should be able to create a branch from an experiment', async () => { + const { experiments } = buildExperiments(disposable) + await experiments.isReady() + + const testExperiment = 'exp-83425' + const mockBranch = 'brunch' + const inputEvent = getInputBoxEvent(mockBranch) + + stub(window, 'showQuickPick').resolves({ + value: { id: testExperiment, name: testExperiment } + } as QuickPickItemWithValue<{ id: string; name: string }>) + + const mockExperimentBranch = stub( + CliExecutor.prototype, + 'experimentBranch' + ).resolves( + `Git branch '${mockBranch}' has been created from experiment '${testExperiment}'. + To switch to the new branch run: + git checkout ${mockBranch}` + ) + + stub( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (WorkspaceExperiments as any).prototype, + 'getOnlyOrPickProject' + ).returns(dvcDemoPath) + + stub(WorkspaceExperiments.prototype, 'getRepository').returns(experiments) + + await commands.executeCommand(RegisteredCliCommands.EXPERIMENT_BRANCH) + + await inputEvent + expect(mockExperimentBranch).to.be.calledWithExactly( + dvcDemoPath, + testExperiment, + mockBranch + ) + }) + }) + + describe('dvc.shareExperimentAsBranch', () => { + it('should be able to share an experiment as a branch', async () => { + const { experiments } = buildExperiments(disposable) + await experiments.isReady() + + const testExperiment = 'exp-83425' + const mockBranch = 'more-brunch' + const inputEvent = getInputBoxEvent(mockBranch) + + stub(window, 'showQuickPick').resolves({ + value: { id: testExperiment, name: testExperiment } + } as QuickPickItemWithValue<{ id: string; name: string }>) + + const mockExperimentBranch = stub( + CliExecutor.prototype, + 'experimentBranch' + ).resolves( + `Git branch '${mockBranch}' has been created from experiment '${testExperiment}'. + To switch to the new branch run: + git checkout ${mockBranch}` + ) + const mockExperimentApply = stub( + CliExecutor.prototype, + 'experimentApply' + ).resolves( + `Changes for experiment '${testExperiment}' have been applied to your current workspace.` + ) + const mockPush = stub(CliExecutor.prototype, 'push').resolves( + '10 files updated.' + ) + const mockGitPush = stub(Git, 'gitPushBranch') + const branchPushedToRemote = new Promise(resolve => + mockGitPush.callsFake(() => { + resolve(undefined) + return Promise.resolve(`${mockBranch} pushed to remote`) + }) + ) + + stub( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (WorkspaceExperiments as any).prototype, + 'getOnlyOrPickProject' + ).returns(dvcDemoPath) + + stub(WorkspaceExperiments.prototype, 'getRepository').returns(experiments) + + await commands.executeCommand( + RegisteredCliCommands.EXPERIMENT_SHARE_AS_BRANCH + ) + + await inputEvent + await branchPushedToRemote + expect(mockExperimentBranch).to.be.calledWithExactly( + dvcDemoPath, + testExperiment, + mockBranch + ) + expect(mockExperimentApply).to.be.calledWithExactly( + dvcDemoPath, + testExperiment + ) + expect(mockPush).to.be.calledWithExactly(dvcDemoPath) + expect(mockGitPush).to.be.calledWithExactly(dvcDemoPath, mockBranch) + }) + }) + describe('dvc.removeExperiment', () => { it('should ask the user to pick an experiment and then remove that experiment from the workspace', async () => { const mockExperiment = 'exp-to-remove'