From 6a2be93f4adc42da822db0b6f3f81fd636d3d487 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Mon, 5 Jun 2023 19:36:44 +1000 Subject: [PATCH] Add default value for branch name when creating from experiment --- .../src/experiments/commands/register.ts | 7 +- extension/src/experiments/workspace.test.ts | 65 ++++++++----------- extension/src/experiments/workspace.ts | 21 ++++-- 3 files changed, 42 insertions(+), 51 deletions(-) diff --git a/extension/src/experiments/commands/register.ts b/extension/src/experiments/commands/register.ts index 82d8aa405e..2d98ba6591 100644 --- a/extension/src/experiments/commands/register.ts +++ b/extension/src/experiments/commands/register.ts @@ -129,11 +129,7 @@ const registerExperimentInputCommands = ( ): void => { internalCommands.registerExternalCliCommand( RegisteredCliCommands.EXPERIMENT_BRANCH, - () => - experiments.getCwdExpNameAndInputThenRun( - getBranchExperimentCommand(experiments), - Title.ENTER_BRANCH_NAME - ) + () => experiments.createExperimentBranch() ) internalCommands.registerExternalCliCommand( @@ -142,6 +138,7 @@ const registerExperimentInputCommands = ( experiments.getInputAndRun( getBranchExperimentCommand(experiments), Title.ENTER_BRANCH_NAME, + `${id}-branch`, dvcRoot, id ) diff --git a/extension/src/experiments/workspace.test.ts b/extension/src/experiments/workspace.test.ts index 400836be6d..5ec68f5c8c 100644 --- a/extension/src/experiments/workspace.test.ts +++ b/extension/src/experiments/workspace.test.ts @@ -16,7 +16,6 @@ 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/dvc/constants' import { findOrCreateDvcYamlFile, getFileExtension, @@ -44,6 +43,7 @@ const mockedHasDvcYamlFile = jest.mocked(hasDvcYamlFile) const mockedGetBranches = jest.fn() const mockedGetCurrentBranch = jest.fn() const mockedPickFile = jest.mocked(pickFile) +const mockedExpBranch = jest.fn() jest.mock('vscode') jest.mock('@hediet/std/disposable') @@ -91,6 +91,11 @@ describe('Experiments', () => { () => mockedGetCurrentBranch() ) + mockedInternalCommands.registerCommand( + AvailableCommands.EXP_BRANCH, + mockedExpBranch + ) + const workspaceExperiments = new WorkspaceExperiments( mockedInternalCommands, buildMockMemento(), @@ -242,79 +247,61 @@ describe('Experiments', () => { }) }) - describe('getCwdExpNameAndInputThenRun', () => { - it('should call the correct function with the correct parameters if a project and experiment are picked and an input provided', async () => { + describe('createExperimentBranch', () => { + it('should create a branch with the correct name if a project and experiment are picked and an input provided', async () => { mockedQuickPickOne.mockResolvedValueOnce(mockedDvcRoot) mockedListStages.mockResolvedValueOnce('train') mockedPickCommitOrExperiment.mockResolvedValueOnce('a123456') mockedGetInput.mockResolvedValueOnce('abc123') - await workspaceExperiments.getCwdExpNameAndInputThenRun( - (cwd: string, ...args: Args) => - workspaceExperiments.runCommand(mockedCommandId, cwd, ...args), - 'enter your password please' as Title - ) + await workspaceExperiments.createExperimentBranch() expect(mockedQuickPickOne).toHaveBeenCalledTimes(1) + expect(mockedGetInput).toHaveBeenCalledWith( + Title.ENTER_BRANCH_NAME, + 'a123456-branch' + ) expect(mockedPickCommitOrExperiment).toHaveBeenCalledTimes(1) - expect(mockedExpFunc).toHaveBeenCalledTimes(1) - expect(mockedExpFunc).toHaveBeenCalledWith( + expect(mockedExpBranch).toHaveBeenCalledTimes(1) + expect(mockedExpBranch).toHaveBeenCalledWith( mockedDvcRoot, 'a123456', 'abc123' ) }) - it('should not call the function or ask for input if a project is not picked', async () => { + it('should not ask for a branch name if a project is not picked', async () => { mockedQuickPickOne.mockResolvedValueOnce(undefined) - await workspaceExperiments.getCwdExpNameAndInputThenRun( - (cwd: string, ...args: Args) => - workspaceExperiments.runCommand(mockedCommandId, cwd, ...args), - 'please name the branch' as Title - ) + await workspaceExperiments.createExperimentBranch() expect(mockedQuickPickOne).toHaveBeenCalledTimes(1) expect(mockedGetInput).not.toHaveBeenCalled() - expect(mockedExpFunc).not.toHaveBeenCalled() + expect(mockedExpBranch).not.toHaveBeenCalled() }) - it('should not call the function if user input is not provided', async () => { + it('should not create a branch if the user does not provide a name', async () => { mockedQuickPickOne.mockResolvedValueOnce(mockedDvcRoot) mockedListStages.mockResolvedValueOnce('train') - mockedPickCommitOrExperiment.mockResolvedValueOnce({ - id: 'b456789', - name: 'exp-456' - }) + mockedPickCommitOrExperiment.mockResolvedValueOnce('exp-456') mockedGetInput.mockResolvedValueOnce(undefined) - await workspaceExperiments.getCwdExpNameAndInputThenRun( - (cwd: string, ...args: Args) => - workspaceExperiments.runCommand(mockedCommandId, cwd, ...args), - 'please enter your bank account number and sort code' as Title - ) + await workspaceExperiments.createExperimentBranch() expect(mockedQuickPickOne).toHaveBeenCalledTimes(1) expect(mockedGetInput).toHaveBeenCalledTimes(1) - expect(mockedExpFunc).not.toHaveBeenCalled() + expect(mockedExpBranch).not.toHaveBeenCalled() }) - it('should check and ask for the creation of a pipeline stage before running the command', async () => { + it('should check and ask for the creation of a pipeline stage before doing anything else', async () => { mockedQuickPickOne.mockResolvedValueOnce(mockedDvcRoot) mockedListStages.mockResolvedValueOnce('') - mockedPickCommitOrExperiment.mockResolvedValueOnce({ - id: 'a123456', - name: 'exp-123' - }) + mockedPickCommitOrExperiment.mockResolvedValueOnce('exp-123') mockedGetInput.mockResolvedValueOnce('abc123') - await workspaceExperiments.getCwdExpNameAndInputThenRun( - (cwd: string, ...args: Args) => - workspaceExperiments.runCommand(mockedCommandId, cwd, ...args), - 'enter your password please' as Title - ) + await workspaceExperiments.createExperimentBranch() - expect(mockedExpFunc).not.toHaveBeenCalled() + expect(mockedExpBranch).not.toHaveBeenCalled() }) }) diff --git a/extension/src/experiments/workspace.ts b/extension/src/experiments/workspace.ts index 73bd4fc80d..c77454b285 100644 --- a/extension/src/experiments/workspace.ts +++ b/extension/src/experiments/workspace.ts @@ -1,7 +1,10 @@ import { EventEmitter, Memento } from 'vscode' import isEmpty from 'lodash.isempty' import { Experiments, ModifiedExperimentAndRunCommandId } from '.' -import { getPushExperimentCommand } from './commands' +import { + getBranchExperimentCommand, + getPushExperimentCommand +} from './commands' import { TableData } from './webview/contract' import { Args } from '../cli/dvc/constants' import { @@ -248,10 +251,7 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews< } } - public async getCwdExpNameAndInputThenRun( - runCommand: (cwd: string, ...args: Args) => Promise, - title: Title - ) { + public async createExperimentBranch() { const cwd = await this.shouldRun() if (!cwd) { return @@ -262,15 +262,22 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews< if (!experimentId) { return } - return this.getInputAndRun(runCommand, title, cwd, experimentId) + return this.getInputAndRun( + getBranchExperimentCommand(this), + Title.ENTER_BRANCH_NAME, + `${experimentId}-branch`, + cwd, + experimentId + ) } public async getInputAndRun( runCommand: (...args: Args) => Promise | void, title: Title, + defaultValue: string, ...args: Args ) { - const input = await getInput(title) + const input = await getInput(title, defaultValue) if (!input) { return }