Skip to content

Commit 8a1492e

Browse files
authored
Show the setup instead of original webview if needed (command palette) (#3256)
* Show the setup instead of original webview if needed (command palette) * Fix test * Use executeCommand and clean up package.json * Try adding tests * Get tests to work * Use registered commands * Make testing easier
1 parent 2350932 commit 8a1492e

File tree

12 files changed

+146
-32
lines changed

12 files changed

+146
-32
lines changed

extension/package.json

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,13 +1353,7 @@
13531353
"viewsWelcome": [
13541354
{
13551355
"view": "dvc.views.webviews",
1356-
"contents": "[Show Experiments](command:dvc.showExperiments)\n[Show Plots](command:dvc.showPlots)\n[Show Experiments and Plots](command:dvc.showExperimentsAndPlots)",
1357-
"when": "dvc.commands.available && dvc.project.available && dvc.project.hasData"
1358-
},
1359-
{
1360-
"view": "dvc.views.webviews",
1361-
"contents": "[Show Experiments](command:dvc.showSetup)\n[Show Plots](command:dvc.showSetup)\n[Show Experiments and Plots](command:dvc.showSetup)",
1362-
"when": "!dvc.commands.available || !dvc.project.available || !dvc.project.hasData"
1356+
"contents": "[Show Experiments](command:dvc.showExperiments)\n[Show Plots](command:dvc.showPlots)\n[Show Experiments and Plots](command:dvc.showExperimentsAndPlots)"
13631357
},
13641358
{
13651359
"view": "dvc.views.welcome",

extension/src/commands/util.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { commands } from 'vscode'
2+
import { RegisteredCommands } from './external'
3+
import { Setup } from '../setup'
4+
import { Context } from '../vscode/context'
5+
6+
export const showSetupOrExecuteCommand =
7+
<T>(setup: Setup, callback: (context: Context) => Promise<T | undefined>) =>
8+
(context: Context) =>
9+
setup.shouldBeShown()
10+
? commands.executeCommand(RegisteredCommands.SETUP_SHOW)
11+
: callback(context)

extension/src/experiments/commands/register.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import {
1212
} from '../../commands/external'
1313
import { Title } from '../../vscode/title'
1414
import { Context, getDvcRootFromContext } from '../../vscode/context'
15+
import { Setup } from '../../setup'
16+
import { showSetupOrExecuteCommand } from '../../commands/util'
1517

1618
type ExperimentDetails = { dvcRoot: string; id: string }
1719

@@ -280,7 +282,8 @@ const registerExperimentQuickPickCommands = (
280282

281283
const registerExperimentRunCommands = (
282284
experiments: WorkspaceExperiments,
283-
internalCommands: InternalCommands
285+
internalCommands: InternalCommands,
286+
setup: Setup
284287
): void => {
285288
internalCommands.registerExternalCliCommand(
286289
RegisteredCliCommands.EXPERIMENT_RUN,
@@ -317,20 +320,22 @@ const registerExperimentRunCommands = (
317320

318321
internalCommands.registerExternalCommand(
319322
RegisteredCommands.EXPERIMENT_SHOW,
320-
(context: Context) =>
323+
showSetupOrExecuteCommand(setup, context =>
321324
experiments.showWebview(getDvcRootFromContext(context))
325+
)
322326
)
323327
}
324328

325329
export const registerExperimentCommands = (
326330
experiments: WorkspaceExperiments,
327-
internalCommands: InternalCommands
331+
internalCommands: InternalCommands,
332+
setup: Setup
328333
) => {
329334
registerExperimentCwdCommands(experiments, internalCommands)
330335
registerExperimentNameCommands(experiments, internalCommands)
331336
registerExperimentInputCommands(experiments, internalCommands)
332337
registerExperimentQuickPickCommands(experiments, internalCommands)
333-
registerExperimentRunCommands(experiments, internalCommands)
338+
registerExperimentRunCommands(experiments, internalCommands, setup)
334339

335340
internalCommands.registerExternalCommand(
336341
RegisteredCommands.EXPERIMENT_AUTO_APPLY_FILTERS,

extension/src/extension.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,19 @@ export class Extension extends Disposable {
185185
)
186186
)
187187

188-
registerExperimentCommands(this.experiments, this.internalCommands)
189-
registerPlotsCommands(this.plots, this.internalCommands)
188+
registerExperimentCommands(
189+
this.experiments,
190+
this.internalCommands,
191+
this.setup
192+
)
193+
registerPlotsCommands(this.plots, this.internalCommands, this.setup)
190194
this.internalCommands.registerExternalCommand(
191195
RegisteredCommands.EXPERIMENT_AND_PLOTS_SHOW,
192196
async (context: VsCodeContext) => {
197+
if (this.setup.shouldBeShown()) {
198+
await commands.executeCommand(RegisteredCommands.SETUP_SHOW)
199+
return
200+
}
193201
const dvcRoot = getDvcRootFromContext(context)
194202
await this.experiments.showWebview(dvcRoot, ViewColumn.Active)
195203
await this.plots.showWebview(dvcRoot, ViewColumn.Beside)

extension/src/plots/commands/register.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
11
import { RegisteredCommands } from '../../commands/external'
22
import { InternalCommands } from '../../commands/internal'
3+
import { showSetupOrExecuteCommand } from '../../commands/util'
4+
import { Setup } from '../../setup'
35
import { Context, getDvcRootFromContext } from '../../vscode/context'
46
import { WorkspacePlots } from '../workspace'
57

68
export const registerPlotsCommands = (
79
plots: WorkspacePlots,
8-
internalCommands: InternalCommands
10+
internalCommands: InternalCommands,
11+
setup: Setup
912
) => {
1013
internalCommands.registerExternalCommand(
1114
RegisteredCommands.PLOTS_SHOW,
12-
(context: Context) => plots.showWebview(getDvcRootFromContext(context))
15+
showSetupOrExecuteCommand(setup, context =>
16+
plots.showWebview(getDvcRootFromContext(context))
17+
)
1318
)
1419

1520
internalCommands.registerExternalCommand(

extension/src/setup/index.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -220,21 +220,20 @@ export class Setup
220220
return this.sendDataToWebview()
221221
}
222222

223-
public async sendDataToWebview() {
224-
const projectInitialized = this.hasRoots()
225-
const hasData = this.getHasData()
223+
public shouldBeShown() {
224+
return !this.cliCompatible || !this.hasRoots() || !this.getHasData()
225+
}
226226

227-
if (
228-
this.webview?.isVisible &&
229-
this.cliCompatible &&
230-
projectInitialized &&
231-
hasData
232-
) {
227+
public async sendDataToWebview() {
228+
if (this.webview?.isVisible && !this.shouldBeShown()) {
233229
this.getWebview()?.dispose()
234230
this.showExperiments()
235231
return
236232
}
237233

234+
const projectInitialized = this.hasRoots()
235+
const hasData = this.getHasData()
236+
238237
const needsGitInitialized =
239238
!projectInitialized && !!(await this.needsGitInit())
240239

extension/src/test/suite/experiments/index.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ import { ConfigKey } from '../../../vscode/config'
7373
import { EXPERIMENT_WORKSPACE_ID } from '../../../cli/dvc/contract'
7474
import * as Time from '../../../util/time'
7575
import { AvailableCommands } from '../../../commands/internal'
76+
import { Setup } from '../../../setup'
7677
import * as FileSystem from '../../../fileSystem'
7778
import * as ProcessExecution from '../../../processExecution'
7879

@@ -1025,6 +1026,8 @@ suite('Experiments Test Suite', () => {
10251026
}).timeout(WEBVIEW_TEST_TIMEOUT)
10261027

10271028
it('should be able to handle a message to compare experiments plots', async () => {
1029+
const mockShouldBeShown = stub(Setup.prototype, 'shouldBeShown')
1030+
mockShouldBeShown.returns(false)
10281031
const { experiments, experimentsModel } = buildExperiments(disposable)
10291032
const mockShowPlots = stub(WorkspacePlots.prototype, 'showWebview')
10301033

extension/src/test/suite/experiments/util.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ export const buildMultiRepoExperiments = (disposer: SafeWatcherDisposer) => {
136136

137137
export const buildSingleRepoExperiments = (disposer: SafeWatcherDisposer) => {
138138
const {
139+
config,
139140
internalCommands,
140141
gitReader,
141142
messageSpy,
@@ -159,7 +160,13 @@ export const buildSingleRepoExperiments = (disposer: SafeWatcherDisposer) => {
159160

160161
void experiments.setState(expShowFixture)
161162

162-
return { messageSpy, workspaceExperiments }
163+
return {
164+
config,
165+
internalCommands,
166+
messageSpy,
167+
resourceLocator,
168+
workspaceExperiments
169+
}
163170
}
164171

165172
const buildExperimentsDataDependencies = (disposer: Disposer) => {

extension/src/test/suite/experiments/workspace.test.ts

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { afterEach, beforeEach, describe, it, suite } from 'mocha'
22
import { expect } from 'chai'
3-
import { stub, restore, SinonStub, match } from 'sinon'
3+
import { stub, restore, SinonStub, match, spy } from 'sinon'
44
import { window, commands, QuickPickItem, Uri } from 'vscode'
55
import {
66
buildExperiments,
@@ -19,7 +19,10 @@ import {
1919
mockDuration
2020
} from '../util'
2121
import { dvcDemoPath } from '../../util'
22-
import { RegisteredCliCommands } from '../../../commands/external'
22+
import {
23+
RegisteredCliCommands,
24+
RegisteredCommands
25+
} from '../../../commands/external'
2326
import * as Telemetry from '../../../telemetry'
2427
import { DvcRunner } from '../../../cli/dvc/runner'
2528
import { Param } from '../../../experiments/model/modify/collect'
@@ -35,6 +38,8 @@ import { GitExecutor } from '../../../cli/git/executor'
3538
import { EXPERIMENT_WORKSPACE_ID } from '../../../cli/dvc/contract'
3639
import { formatDate } from '../../../util/date'
3740
import { DvcReader } from '../../../cli/dvc/reader'
41+
import { Setup } from '../../../setup'
42+
import { WorkspaceExperiments } from '../../../experiments/workspace'
3843

3944
suite('Workspace Experiments Test Suite', () => {
4045
const disposable = getTimeSafeDisposer()
@@ -932,4 +937,52 @@ suite('Workspace Experiments Test Suite', () => {
932937
expect(mockExperimentRemove).to.be.calledWith(dvcDemoPath, '--queue')
933938
})
934939
})
940+
941+
describe('dvc.showExperiments', () => {
942+
it('should show the setup if it should be shown', async () => {
943+
const executeCommandSpy = spy(commands, 'executeCommand')
944+
945+
stub(Setup.prototype, 'shouldBeShown').returns(true)
946+
947+
await commands.executeCommand(RegisteredCommands.EXPERIMENT_SHOW)
948+
949+
expect(executeCommandSpy).to.have.been.calledWithMatch('dvc.showSetup')
950+
})
951+
952+
it('should not show the experiments webview if the setup should be shown', async () => {
953+
const showPlotsWebviewSpy = stub(
954+
WorkspaceExperiments.prototype,
955+
'showWebview'
956+
)
957+
stub(Setup.prototype, 'shouldBeShown').returns(true)
958+
959+
await commands.executeCommand(RegisteredCommands.EXPERIMENT_SHOW)
960+
961+
expect(showPlotsWebviewSpy).not.to.be.called
962+
})
963+
964+
it('should not show the setup if it should not be shown', async () => {
965+
const executeCommandSpy = spy(commands, 'executeCommand')
966+
967+
stub(WorkspaceExperiments.prototype, 'showWebview').resolves()
968+
969+
stub(Setup.prototype, 'shouldBeShown').returns(false)
970+
971+
await commands.executeCommand(RegisteredCommands.EXPERIMENT_SHOW)
972+
973+
expect(executeCommandSpy).not.to.be.calledWith('dvc.showSetup')
974+
})
975+
976+
it('should show the experiments webview if the setup should not be shown', async () => {
977+
const showPlotsWebviewSpy = stub(
978+
WorkspaceExperiments.prototype,
979+
'showWebview'
980+
).resolves()
981+
stub(Setup.prototype, 'shouldBeShown').returns(false)
982+
983+
await commands.executeCommand(RegisteredCommands.EXPERIMENT_SHOW)
984+
985+
expect(showPlotsWebviewSpy).to.be.called
986+
})
987+
})
935988
})

extension/src/test/suite/plots/util.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,23 @@ export const buildPlots = async (
109109
}
110110
}
111111

112+
export const buildWorkspacePlots = (disposer: Disposer) => {
113+
const { config, internalCommands, messageSpy, resourceLocator } =
114+
buildDependencies(disposer)
115+
116+
const workspacePlots = disposer.track(
117+
new WorkspacePlots(internalCommands, buildMockMemento())
118+
)
119+
120+
return {
121+
config,
122+
internalCommands,
123+
messageSpy,
124+
resourceLocator,
125+
workspacePlots
126+
}
127+
}
128+
112129
export const getExpectedCheckpointPlotsData = (
113130
domain: string[],
114131
range: Color[]

0 commit comments

Comments
 (0)