-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge create plot commands #4680
Changes from 3 commits
b2f6d02
d7d3f5f
955edf5
9311223
68ff206
403e93f
6233c89
ec3c5ed
3329859
0aeb1cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
import { commands } from 'vscode' | ||
import { RegisteredCommands } from './external' | ||
import { addPlotCommand } from './util' | ||
import { quickPickValue } from '../vscode/quickPick' | ||
import { Title } from '../vscode/title' | ||
|
||
jest.mock('../vscode/quickPick') | ||
|
||
const mockedQuickPickValue = jest.mocked(quickPickValue) | ||
const mockedCommands = jest.mocked(commands) | ||
const mockedExecuteCommand = jest.fn() | ||
mockedCommands.executeCommand = mockedExecuteCommand | ||
|
||
beforeEach(() => { | ||
jest.resetAllMocks() | ||
}) | ||
|
||
describe('addPlotCommand', () => { | ||
it('should call no add commands if no plots types are selected', async () => { | ||
mockedQuickPickValue.mockResolvedValueOnce(undefined) | ||
|
||
await addPlotCommand(undefined) | ||
|
||
expect(mockedQuickPickValue).toHaveBeenCalledWith( | ||
[ | ||
{ | ||
description: 'Create a dvc.yaml plot based off a chosen data file', | ||
label: 'Top-Level', | ||
value: 'top-level' | ||
}, | ||
{ | ||
description: | ||
'Create an extension-only plot based off a chosen metric and param', | ||
label: 'Custom', | ||
value: 'custom' | ||
} | ||
], | ||
{ | ||
title: Title.SELECT_PLOT_TYPE | ||
} | ||
) | ||
expect(mockedExecuteCommand).not.toHaveBeenCalled() | ||
}) | ||
|
||
it('should call the add top-level plot command if the top-level type is selected', async () => { | ||
mockedQuickPickValue.mockResolvedValueOnce('top-level') | ||
|
||
await addPlotCommand(undefined) | ||
|
||
expect(mockedExecuteCommand).toHaveBeenCalledWith( | ||
RegisteredCommands.PIPELINE_ADD_PLOT, | ||
undefined | ||
) | ||
}) | ||
|
||
it('should call the add custom plot command if the custom type is selected', async () => { | ||
mockedQuickPickValue.mockResolvedValueOnce('custom') | ||
|
||
await addPlotCommand('cwd') | ||
|
||
expect(mockedExecuteCommand).toHaveBeenCalledWith( | ||
RegisteredCommands.PLOTS_CUSTOM_ADD, | ||
'cwd' | ||
) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,8 @@ import { commands } from 'vscode' | |
import { RegisteredCommands } from './external' | ||
import { Setup } from '../setup' | ||
import { Context } from '../vscode/context' | ||
import { quickPickValue } from '../vscode/quickPick' | ||
import { Title } from '../vscode/title' | ||
|
||
export const showSetupOrExecuteCommand = | ||
<T>(setup: Setup, callback: (context: Context) => Promise<T | undefined>) => | ||
|
@@ -18,3 +20,40 @@ export const showSetupOrExecuteCommand = | |
|
||
return callback(context) | ||
} | ||
|
||
const enum PLOT_TYPE { | ||
CUSTOM = 'custom', | ||
TOP_LEVEL = 'top-level' | ||
} | ||
|
||
export const addPlotCommand = async (context: Context) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [I] These should go in a new There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Oh, I think we would get two since |
||
const result: PLOT_TYPE | undefined = await quickPickValue( | ||
julieg18 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
[ | ||
{ | ||
description: 'Create a dvc.yaml plot based off a chosen data file', | ||
julieg18 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
label: 'Top-Level', | ||
value: PLOT_TYPE.TOP_LEVEL | ||
}, | ||
{ | ||
description: | ||
'Create an extension-only plot based off a chosen metric and param', | ||
julieg18 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
label: 'Custom', | ||
value: PLOT_TYPE.CUSTOM | ||
} | ||
], | ||
{ | ||
title: Title.SELECT_PLOT_TYPE | ||
} | ||
) | ||
|
||
if (result === PLOT_TYPE.CUSTOM) { | ||
return commands.executeCommand(RegisteredCommands.PLOTS_CUSTOM_ADD, context) | ||
} | ||
|
||
if (result === PLOT_TYPE.TOP_LEVEL) { | ||
return commands.executeCommand( | ||
RegisteredCommands.PIPELINE_ADD_PLOT, | ||
context | ||
) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a better place for these two commands that are registered in
extension.ts
? I couldn't think of a place that didn't feel confusing 🤔