diff --git a/extension/package.json b/extension/package.json index 7643b159ea..438488a072 100644 --- a/extension/package.json +++ b/extension/package.json @@ -460,6 +460,12 @@ "category": "DVC", "icon": "$(play)" }, + { + "title": "%command.views.experiments.shareExperimentAsBranch%", + "command": "dvc.views.experiments.shareExperimentAsBranch", + "category": "DVC", + "icon": "$(repo-push)" + }, { "title": "%command.views.experimentsTree.selectExperiments%", "command": "dvc.views.experimentsTree.selectExperiments", @@ -783,6 +789,10 @@ "command": "dvc.views.experiments.resetAndRunCheckpointExperiment", "when": "false" }, + { + "command": "dvc.views.experiments.shareExperimentAsBranch", + "when": "false" + }, { "command": "dvc.views.experimentsFilterByTree.removeAllFilters", "when": "false" @@ -1069,24 +1079,29 @@ "group": "inline@3", "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(experiment|queued)$/ && !dvc.experiment.running" }, + { + "command": "dvc.views.experiments.shareExperimentAsBranch", + "group": "1_share@1", + "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(checkpoint|experiment)$/ && !dvc.experiment.running" + }, { "command": "dvc.views.experiments.runExperiment", - "group": "1_modify@1", + "group": "2_modify@1", "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.experiment.running && !dvc.experiment.checkpoints" }, { "command": "dvc.views.experiments.resetAndRunCheckpointExperiment", - "group": "1_modify@1", + "group": "2_modify@1", "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.experiment.running && dvc.experiment.checkpoints" }, { "command": "dvc.views.experiments.resumeCheckpointExperiment", - "group": "1_modify@2", + "group": "2_modify@2", "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.experiment.running && dvc.experiment.checkpoints" }, { "command": "dvc.views.experiments.queueExperiment", - "group": "1_modify@3", + "group": "2_modify@3", "when": "view == dvc.views.experimentsTree && dvc.commands.available && viewItem =~ /^(workspace|branch|experiment|queued)$/ && !dvc.experiment.running" }, { diff --git a/extension/package.nls.json b/extension/package.nls.json index 846872c0b6..0de3f478e7 100644 --- a/extension/package.nls.json +++ b/extension/package.nls.json @@ -67,6 +67,7 @@ "command.views.experiments.runExperiment": "Modify Param(s) and Run", "command.views.experiments.resumeCheckpointExperiment": "Modify Param(s) and Resume", "command.views.experiments.resetAndRunCheckpointExperiment": "Modify Param(s), Reset and Run", + "command.views.experiments.shareExperimentAsBranch": "Share as Branch", "command.views.experimentsTree.selectExperiments": "Select Experiments to Display in Plots", "command.views.plotsPathsTree.selectPlots": "Select Plots to Display", "command.views.plotsPathsTree.refreshPlots": "Refresh Plots for Selected Experiments", diff --git a/extension/src/commands/external.ts b/extension/src/commands/external.ts index b11ff4acad..1708b11e5d 100644 --- a/extension/src/commands/external.ts +++ b/extension/src/commands/external.ts @@ -14,6 +14,7 @@ export enum RegisteredCliCommands { EXPERIMENT_VIEW_APPLY = 'dvc.views.experiments.applyExperiment', EXPERIMENT_VIEW_BRANCH = 'dvc.views.experiments.branchExperiment', EXPERIMENT_VIEW_REMOVE = 'dvc.views.experiments.removeExperiment', + EXPERIMENT_VIEW_SHARE_AS_BRANCH = 'dvc.views.experiments.shareExperimentAsBranch', EXPERIMENT_VIEW_QUEUE = 'dvc.views.experiments.queueExperiment', EXPERIMENT_VIEW_RESUME = 'dvc.views.experiments.resumeCheckpointExperiment', diff --git a/extension/src/experiments/commands/register.ts b/extension/src/experiments/commands/register.ts index 1367883ccd..4668560379 100644 --- a/extension/src/experiments/commands/register.ts +++ b/extension/src/experiments/commands/register.ts @@ -6,6 +6,9 @@ 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 } @@ -153,7 +156,12 @@ const registerExperimentInputCommands = ( RegisteredCliCommands.EXPERIMENT_BRANCH, () => experiments.getCwdExpNameAndInputThenRun( - AvailableCommands.EXPERIMENT_BRANCH, + (cwd, ...args: Args) => + experiments.runCommand( + AvailableCommands.EXPERIMENT_BRANCH, + cwd, + ...args + ), Title.ENTER_BRANCH_NAME ) ) @@ -162,7 +170,38 @@ const registerExperimentInputCommands = ( RegisteredCliCommands.EXPERIMENT_VIEW_BRANCH, ({ dvcRoot, id }: ExperimentDetails) => experiments.getExpNameAndInputThenRun( - AvailableCommands.EXPERIMENT_BRANCH, + (name: string, input: string) => + experiments.runCommand( + AvailableCommands.EXPERIMENT_BRANCH, + dvcRoot, + name, + input + ), + Title.ENTER_BRANCH_NAME, + dvcRoot, + id + ) + ) + + internalCommands.registerExternalCliCommand( + 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)) + }, Title.ENTER_BRANCH_NAME, dvcRoot, id diff --git a/extension/src/experiments/webview/messages.ts b/extension/src/experiments/webview/messages.ts index 2107e4a0c8..ad8a841332 100644 --- a/extension/src/experiments/webview/messages.ts +++ b/extension/src/experiments/webview/messages.ts @@ -116,6 +116,12 @@ export class WebviewMessages { case MessageFromWebviewType.OPEN_PLOTS_WEBVIEW: return commands.executeCommand(RegisteredCommands.PLOTS_SHOW) + case MessageFromWebviewType.SHARE_EXPERIMENT_AS_BRANCH: + return commands.executeCommand( + RegisteredCliCommands.EXPERIMENT_VIEW_SHARE_AS_BRANCH, + { dvcRoot: this.dvcRoot, id: message.payload } + ) + default: Logger.error(`Unexpected message: ${JSON.stringify(message)}`) } diff --git a/extension/src/experiments/workspace.test.ts b/extension/src/experiments/workspace.test.ts index 29edf4e0fa..a6016fa5c1 100644 --- a/extension/src/experiments/workspace.test.ts +++ b/extension/src/experiments/workspace.test.ts @@ -12,6 +12,7 @@ import { buildMockMemento } from '../test/util' import { buildMockedEventEmitter } from '../test/util/jest' import { OutputChannel } from '../vscode/outputChannel' import { Title } from '../vscode/title' +import { Args } from '../cli/constants' const mockedShowWebview = jest.fn() const mockedDisposable = jest.mocked(Disposable) @@ -185,7 +186,7 @@ describe('Experiments', () => { }) }) - describe('getExpNameAndInputThenRun', () => { + describe('getCwdExpNameAndInputThenRun', () => { it('should call the correct function with the correct parameters if a project and experiment are picked and an input provided', async () => { mockedQuickPickOne.mockResolvedValueOnce(mockedDvcRoot) mockedPickCurrentExperiment.mockResolvedValueOnce({ @@ -195,7 +196,8 @@ describe('Experiments', () => { mockedGetInput.mockResolvedValueOnce('abc123') await workspaceExperiments.getCwdExpNameAndInputThenRun( - mockedCommandId, + (cwd: string, ...args: Args) => + workspaceExperiments.runCommand(mockedCommandId, cwd, ...args), 'enter your password please' as Title ) @@ -209,7 +211,8 @@ describe('Experiments', () => { mockedQuickPickOne.mockResolvedValueOnce(undefined) await workspaceExperiments.getCwdExpNameAndInputThenRun( - mockedCommandId, + (cwd: string, ...args: Args) => + workspaceExperiments.runCommand(mockedCommandId, cwd, ...args), 'please name the branch' as Title ) @@ -227,7 +230,8 @@ describe('Experiments', () => { mockedGetInput.mockResolvedValueOnce(undefined) await workspaceExperiments.getCwdExpNameAndInputThenRun( - mockedCommandId, + (cwd: string, ...args: Args) => + workspaceExperiments.runCommand(mockedCommandId, cwd, ...args), 'please enter your bank account number and sort code' as Title ) diff --git a/extension/src/experiments/workspace.ts b/extension/src/experiments/workspace.ts index 0b27f66c3b..071161d5cd 100644 --- a/extension/src/experiments/workspace.ts +++ b/extension/src/experiments/workspace.ts @@ -222,10 +222,10 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews< } } - public getCwdExpNameAndInputThenRun = async ( - commandId: CommandId, + public async getCwdExpNameAndInputThenRun( + runCommand: (cwd: string, ...args: Args) => Promise, title: Title - ) => { + ) { const cwd = await this.getFocusedOrOnlyOrPickProject() if (!cwd) { return @@ -236,11 +236,11 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews< if (!experiment) { return } - return this.getInputAndRun(commandId, cwd, title, experiment.name) + return this.getInputAndRun(runCommand, title, cwd, experiment.name) } public getExpNameAndInputThenRun( - commandId: CommandId, + runCommand: (...args: Args) => Promise, title: Title, cwd: string, id: string @@ -251,20 +251,7 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews< return } - return this.getInputAndRun(commandId, cwd, title, name) - } - - public async getInputAndRun( - commandId: CommandId, - cwd: string, - title: Title, - ...args: Args - ) { - const input = await getInput(title) - if (!input) { - return - } - return this.runCommand(commandId, cwd, ...args, input) + return this.getInputAndRun(runCommand, title, name) } public getExpNameThenRun(commandId: CommandId, cwd: string, id: string) { @@ -360,6 +347,18 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews< return this.runCommand(commandId, cwd, experiment.name) } + private async getInputAndRun( + runCommand: (...args: Args) => Promise, + title: Title, + ...args: Args + ) { + const input = await getInput(title) + if (!input) { + return + } + return runCommand(...args, input) + } + private pickCurrentExperiment(cwd: string) { return this.getRepository(cwd).pickCurrentExperiment() } diff --git a/extension/src/git.ts b/extension/src/git.ts index 834255005a..45970b09ed 100644 --- a/extension/src/git.ts +++ b/extension/src/git.ts @@ -95,3 +95,13 @@ export const gitStageAll = async (cwd: string) => { executable: 'git' }) } + +export const gitPushBranch = ( + cwd: string, + branchName: string +): Promise => + executeProcess({ + args: ['push', '-u', 'origin', branchName], + cwd, + executable: 'git' + }) diff --git a/extension/src/telemetry/constants.ts b/extension/src/telemetry/constants.ts index d1c12b37b8..ec6abc47fa 100644 --- a/extension/src/telemetry/constants.ts +++ b/extension/src/telemetry/constants.ts @@ -135,6 +135,7 @@ export interface IEventNamePropertyMapping { [EventName.EXPERIMENT_VIEW_APPLY]: undefined [EventName.EXPERIMENT_VIEW_BRANCH]: undefined [EventName.EXPERIMENT_VIEW_REMOVE]: undefined + [EventName.EXPERIMENT_VIEW_SHARE_AS_BRANCH]: undefined [EventName.EXPERIMENT_TOGGLE]: undefined [EventName.EXPERIMENT_VIEW_QUEUE]: undefined diff --git a/extension/src/test/suite/experiments/index.test.ts b/extension/src/test/suite/experiments/index.test.ts index f51304c73f..168402925b 100644 --- a/extension/src/test/suite/experiments/index.test.ts +++ b/extension/src/test/suite/experiments/index.test.ts @@ -63,6 +63,8 @@ import { Title } from '../../../vscode/title' import { ExperimentFlag } from '../../../cli/constants' import { WorkspaceExperiments } from '../../../experiments/workspace' import { CliExecutor } from '../../../cli/executor' +import * as Git from '../../../git' +import { shortenForLabel } from '../../../util/string' suite('Experiments Test Suite', () => { const disposable = Disposable.fn() @@ -475,6 +477,65 @@ suite('Experiments Test Suite', () => { ) }).timeout(WEBVIEW_TEST_TIMEOUT) + it('should handle a message to share an experiment as a new branch', async () => { + const { experiments } = buildExperiments(disposable) + await experiments.isReady() + + const testCheckpointId = 'd1343a87c6ee4a2e82d19525964d2fb2cb6756c9' + const testCheckpointLabel = shortenForLabel(testCheckpointId) + const mockBranch = 'it-is-a-branch-shared-to-the-remote' + const inputEvent = getInputBoxEvent(mockBranch) + + const mockExperimentBranch = stub( + CliExecutor.prototype, + 'experimentBranch' + ).resolves( + `Git branch '${mockBranch}' has been created from experiment '${testCheckpointId}'. + To switch to the new branch run: + git checkout ${mockBranch}` + ) + const mockExperimentApply = stub( + CliExecutor.prototype, + 'experimentApply' + ).resolves( + `Changes for experiment '${testCheckpointId}' 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(WorkspaceExperiments.prototype, 'getRepository').returns(experiments) + + const webview = await experiments.showWebview() + const mockMessageReceived = getMessageReceivedEmitter(webview) + + mockMessageReceived.fire({ + payload: testCheckpointId, + type: MessageFromWebviewType.SHARE_EXPERIMENT_AS_BRANCH + }) + + await inputEvent + await branchPushedToRemote + expect(mockExperimentBranch).to.be.calledWithExactly( + dvcDemoPath, + testCheckpointLabel, + mockBranch + ) + expect(mockExperimentApply).to.be.calledWithExactly( + dvcDemoPath, + testCheckpointLabel + ) + expect(mockPush).to.be.calledWithExactly(dvcDemoPath) + expect(mockGitPush).to.be.calledWithExactly(dvcDemoPath, mockBranch) + }).timeout(WEBVIEW_TEST_TIMEOUT) + it("should be able to handle a message to modify an experiment's params and queue an experiment", async () => { const { experiments, cliExecutor } = buildExperiments(disposable) diff --git a/extension/src/webview/contract.ts b/extension/src/webview/contract.ts index 9b5ef106cc..dff05bb0a5 100644 --- a/extension/src/webview/contract.ts +++ b/extension/src/webview/contract.ts @@ -35,6 +35,7 @@ export enum MessageFromWebviewType { SELECT_EXPERIMENTS = 'select-experiments', SELECT_COLUMNS = 'select-columns', SELECT_PLOTS = 'select-plots', + SHARE_EXPERIMENT_AS_BRANCH = 'share-experiment-as-branch', TOGGLE_METRIC = 'toggle-metric', TOGGLE_PLOTS_SECTION = 'toggle-plots-section', VARY_EXPERIMENT_PARAMS_AND_QUEUE = 'vary-experiment-params-and-queue', @@ -146,6 +147,10 @@ export type MessageFromWebview = | { type: MessageFromWebviewType.FOCUS_FILTERS_TREE } | { type: MessageFromWebviewType.FOCUS_SORTS_TREE } | { type: MessageFromWebviewType.OPEN_PLOTS_WEBVIEW } + | { + type: MessageFromWebviewType.SHARE_EXPERIMENT_AS_BRANCH + payload: string + } export type MessageToWebview = { type: MessageToWebviewType.SET_DATA diff --git a/webview/src/experiments/components/App.test.tsx b/webview/src/experiments/components/App.test.tsx index e5501b68c2..6303e500d9 100644 --- a/webview/src/experiments/components/App.test.tsx +++ b/webview/src/experiments/components/App.test.tsx @@ -679,6 +679,27 @@ describe('App', () => { ]) }) + it('should present the correct option for an experiment with checkpoints', () => { + renderTableWithoutRunningExperiments() + + const target = screen.getByText('[exp-e7a67]') + fireEvent.contextMenu(target, { bubbles: true }) + + jest.advanceTimersByTime(100) + const menuitems = screen.getAllByRole('menuitem') + const itemLabels = menuitems.map(item => item.textContent) + expect(itemLabels).toStrictEqual([ + 'Apply to Workspace', + 'Create new Branch', + 'Share as Branch', + 'Modify, Reset and Run', + 'Modify and Resume', + 'Modify and Queue', + 'Star Experiment', + 'Remove' + ]) + }) + it('should present the Remove experiment option for the checkpoint tips', () => { renderTableWithoutRunningExperiments() diff --git a/webview/src/experiments/components/table/Row.tsx b/webview/src/experiments/components/table/Row.tsx index 3c5c4004f3..5c76b4c404 100644 --- a/webview/src/experiments/components/table/Row.tsx +++ b/webview/src/experiments/components/table/Row.tsx @@ -115,12 +115,11 @@ const getRunResumeOptions = ( hidden?: boolean, divider?: boolean ) => MessagesMenuOptionProps, - isWorkspace: boolean, projectHasCheckpoints: boolean, hideVaryAndRun: boolean, depth: number ) => { - const isNotCheckpoint = depth <= 1 || isWorkspace + const isCheckpoint = depth > 1 const resetNeedsSeparator = !hideVaryAndRun && projectHasCheckpoints const runNeedsSeparator = !hideVaryAndRun && !projectHasCheckpoints @@ -129,19 +128,19 @@ const getRunResumeOptions = ( withId( 'Modify, Reset and Run', MessageFromWebviewType.VARY_EXPERIMENT_PARAMS_RESET_AND_RUN, - !isNotCheckpoint || !projectHasCheckpoints, + isCheckpoint || !projectHasCheckpoints, resetNeedsSeparator ), withId( projectHasCheckpoints ? 'Modify and Resume' : 'Modify and Run', MessageFromWebviewType.VARY_EXPERIMENT_PARAMS_AND_RUN, - !isNotCheckpoint, + isCheckpoint, runNeedsSeparator ), withId( 'Modify and Queue', MessageFromWebviewType.VARY_EXPERIMENT_PARAMS_AND_QUEUE, - !isNotCheckpoint + isCheckpoint ) ] } @@ -155,7 +154,7 @@ const getSingleSelectMenuOptions = ( queued?: boolean, starred?: boolean ) => { - const hideApplyAndCreateBranch = queued || isWorkspace || depth <= 0 + const isNotExperimentOrCheckpoint = queued || isWorkspace || depth <= 0 const withId = ( label: string, @@ -175,18 +174,22 @@ const getSingleSelectMenuOptions = ( withId( 'Apply to Workspace', MessageFromWebviewType.APPLY_EXPERIMENT_TO_WORKSPACE, - hideApplyAndCreateBranch + isNotExperimentOrCheckpoint ), withId( 'Create new Branch', MessageFromWebviewType.CREATE_BRANCH_FROM_EXPERIMENT, - hideApplyAndCreateBranch + isNotExperimentOrCheckpoint + ), + withId( + 'Share as Branch', + MessageFromWebviewType.SHARE_EXPERIMENT_AS_BRANCH, + isNotExperimentOrCheckpoint ), ...getRunResumeOptions( withId, - isWorkspace, projectHasCheckpoints, - hideApplyAndCreateBranch, + isNotExperimentOrCheckpoint, depth ), experimentMenuOption(