Skip to content

Commit 0fce8e1

Browse files
mattseddonmattseddon
authored
Refactor stubWorkspaceExperimentsGetters (#4272)
1 parent ad00496 commit 0fce8e1

File tree

6 files changed

+87
-164
lines changed

6 files changed

+87
-164
lines changed

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

Lines changed: 17 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,7 @@ suite('Experiments Test Suite', () => {
533533
}).timeout(WEBVIEW_TEST_TIMEOUT)
534534

535535
it('should be able to handle a message to apply an experiment', async () => {
536-
const { experiments } = buildExperiments({ disposer: disposable })
536+
const { experiments } = stubWorkspaceExperimentsGetters(disposable)
537537
await experiments.isReady()
538538

539539
const webview = await experiments.showWebview()
@@ -545,8 +545,6 @@ suite('Experiments Test Suite', () => {
545545
'expApply'
546546
).resolves(undefined)
547547

548-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
549-
550548
mockMessageReceived.fire({
551549
payload: mockExperimentId,
552550
type: MessageFromWebviewType.APPLY_EXPERIMENT_TO_WORKSPACE
@@ -560,7 +558,7 @@ suite('Experiments Test Suite', () => {
560558
}).timeout(WEBVIEW_TEST_TIMEOUT)
561559

562560
it('should be able to handle a message to create a branch from an experiment', async () => {
563-
const { experiments } = buildExperiments({ disposer: disposable })
561+
const { experiments } = stubWorkspaceExperimentsGetters(disposable)
564562
await experiments.isReady()
565563

566564
const mockBranch = 'mock-branch-input'
@@ -571,8 +569,6 @@ suite('Experiments Test Suite', () => {
571569
'expBranch'
572570
).resolves('undefined')
573571

574-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
575-
576572
const webview = await experiments.showWebview()
577573
const mockMessageReceived = getMessageReceivedEmitter(webview)
578574
const mockExperimentId = 'exp-e7a67'
@@ -714,9 +710,8 @@ suite('Experiments Test Suite', () => {
714710
}).timeout(WEBVIEW_TEST_TIMEOUT)
715711

716712
it('should be able to handle a message to modify the workspace params and queue an experiment', async () => {
717-
const { experiments, dvcExecutor } = buildExperiments({
718-
disposer: disposable
719-
})
713+
const { experiments, dvcExecutor } =
714+
stubWorkspaceExperimentsGetters(disposable)
720715

721716
const mockModifiedParams = [
722717
'-S',
@@ -725,8 +720,6 @@ suite('Experiments Test Suite', () => {
725720
'params.yaml:weight_decay=0'
726721
]
727722

728-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
729-
730723
stub(experiments, 'pickAndModifyParams').resolves(mockModifiedParams)
731724
const mockQueueExperiment = stub(dvcExecutor, 'expRunQueue').resolves(
732725
undefined
@@ -749,9 +742,8 @@ suite('Experiments Test Suite', () => {
749742
}).timeout(WEBVIEW_TEST_TIMEOUT)
750743

751744
it('should be able to handle a message to modify the workspace params and run a new experiment', async () => {
752-
const { experiments, dvcRunner } = buildExperiments({
753-
disposer: disposable
754-
})
745+
const { experiments, dvcRunner } =
746+
stubWorkspaceExperimentsGetters(disposable)
755747

756748
const mockModifiedParams = [
757749
'-S',
@@ -760,8 +752,6 @@ suite('Experiments Test Suite', () => {
760752
'params.yaml:weight_decay=0'
761753
]
762754

763-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
764-
765755
stub(experiments, 'pickAndModifyParams').resolves(mockModifiedParams)
766756
const mockRunExperiment = stub(dvcRunner, 'runExperiment').resolves(
767757
undefined
@@ -786,9 +776,8 @@ suite('Experiments Test Suite', () => {
786776
}).timeout(WEBVIEW_TEST_TIMEOUT)
787777

788778
it('should be able to handle a message to modify the workspace params, reset and run a new experiment', async () => {
789-
const { experiments, dvcRunner } = buildExperiments({
790-
disposer: disposable
791-
})
779+
const { experiments, dvcRunner } =
780+
stubWorkspaceExperimentsGetters(disposable)
792781

793782
const mockModifiedParams = [
794783
'-S',
@@ -799,8 +788,6 @@ suite('Experiments Test Suite', () => {
799788

800789
stub(experiments, 'pickAndModifyParams').resolves(mockModifiedParams)
801790

802-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
803-
804791
const webview = await experiments.showWebview()
805792
const mockMessageReceived = getMessageReceivedEmitter(webview)
806793
const mockRunExperiment = stub(dvcRunner, 'runExperiment').resolves(
@@ -847,9 +834,8 @@ suite('Experiments Test Suite', () => {
847834
}).timeout(WEBVIEW_TEST_TIMEOUT)
848835

849836
it("should be able to handle a message to toggle an experiment's status", async () => {
850-
const { experiments, experimentsModel } = buildExperiments({
851-
disposer: disposable
852-
})
837+
const { experiments, experimentsModel } =
838+
stubWorkspaceExperimentsGetters(disposable)
853839

854840
await experiments.isReady()
855841

@@ -875,8 +861,6 @@ suite('Experiments Test Suite', () => {
875861
'queued experiment cannot be selected'
876862
).to.be.false
877863

878-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
879-
880864
const webview = await experiments.showWebview()
881865
const mockMessageReceived = getMessageReceivedEmitter(webview)
882866
const toggleSpy = spy(experimentsModel, 'toggleStatus')
@@ -1192,9 +1176,8 @@ suite('Experiments Test Suite', () => {
11921176
}).timeout(WEBVIEW_TEST_TIMEOUT)
11931177

11941178
it('should be able to handle a message to select experiments for plotting', async () => {
1195-
const { experiments, experimentsModel } = buildExperiments({
1196-
disposer: disposable
1197-
})
1179+
const { experiments, experimentsModel } =
1180+
stubWorkspaceExperimentsGetters(disposable)
11981181
await experiments.isReady()
11991182

12001183
const webview = await experiments.showWebview()
@@ -1204,8 +1187,6 @@ suite('Experiments Test Suite', () => {
12041187
const expectedIds = ['main', 'test-branch', runningInQueueId]
12051188
const mockExperimentIds = [...expectedIds, queuedId]
12061189

1207-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
1208-
12091190
const tableChangePromise = experimentsUpdatedEvent(experiments)
12101191

12111192
mockMessageReceived.fire({
@@ -1231,9 +1212,8 @@ suite('Experiments Test Suite', () => {
12311212
dvc: true,
12321213
experiments: true
12331214
})
1234-
const { experiments, experimentsModel } = buildExperiments({
1235-
disposer: disposable
1236-
})
1215+
const { experiments, experimentsModel } =
1216+
stubWorkspaceExperimentsGetters(disposable)
12371217
const mockShowPlots = stub(WorkspacePlots.prototype, 'showWebview')
12381218

12391219
const dataSent = new Promise(resolve =>
@@ -1250,8 +1230,6 @@ suite('Experiments Test Suite', () => {
12501230
const runningInQueueId = 'exp-e7a67'
12511231
const mockExperimentIds = ['main', 'test-branch', runningInQueueId]
12521232

1253-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
1254-
12551233
const tableChangePromise = experimentsUpdatedEvent(experiments)
12561234

12571235
mockMessageReceived.fire({
@@ -1302,9 +1280,9 @@ suite('Experiments Test Suite', () => {
13021280
}).timeout(WEBVIEW_TEST_TIMEOUT)
13031281

13041282
it('should handle a message to stop experiments running', async () => {
1305-
const { experiments, dvcExecutor } = buildExperiments({
1306-
disposer: disposable
1307-
})
1283+
const { experiments, dvcExecutor } =
1284+
stubWorkspaceExperimentsGetters(disposable)
1285+
13081286
const mockQueueKill = stub(dvcExecutor, 'queueKill')
13091287
const mockStopProcesses = stub(ProcessExecution, 'stopProcesses')
13101288

@@ -1328,7 +1306,6 @@ suite('Experiments Test Suite', () => {
13281306
const mockMessageReceived = getMessageReceivedEmitter(webview)
13291307
const mockExperimentIds = ['exp-e7a67', 'exp-83425']
13301308

1331-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
13321309
const mockPid = 1234
13331310
const mockGetPidFromFile = stub(FileSystem, 'getPidFromFile')
13341311
.onFirstCall()

‎extension/src/test/suite/experiments/model/filterBy/tree.test.ts‎

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,11 @@ suite('Experiments Filter By Tree Test Suite', () => {
6060
})
6161

6262
it('should be able to update the table data by adding and removing a filter', async () => {
63-
const { experiments, messageSpy } = buildExperiments({
64-
disposer: disposable
65-
})
66-
63+
const { experiments, messageSpy } =
64+
stubWorkspaceExperimentsGetters(disposable)
6765
await experiments.isReady()
6866
await experiments.showWebview()
6967

70-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
71-
7268
const accuracyPath = buildMetricOrParamPath(
7369
ColumnType.METRICS,
7470
'summary.json',
@@ -135,14 +131,11 @@ suite('Experiments Filter By Tree Test Suite', () => {
135131
}).timeout(WEBVIEW_TEST_TIMEOUT)
136132

137133
it('should be able to remove all filters with dvc.views.experimentsFilterByTree.removeAllFilters', async () => {
138-
const mockShowQuickPick = stub(window, 'showQuickPick')
139-
const mockShowInputBox = stub(window, 'showInputBox')
140-
141-
const { experiments } = buildExperiments({ disposer: disposable })
142-
134+
const { experiments } = stubWorkspaceExperimentsGetters(disposable)
143135
await experiments.isReady()
144136

145-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
137+
const mockShowQuickPick = stub(window, 'showQuickPick')
138+
const mockShowInputBox = stub(window, 'showInputBox')
146139

147140
const lossPath = buildMetricOrParamPath(
148141
ColumnType.METRICS,
@@ -338,15 +331,11 @@ suite('Experiments Filter By Tree Test Suite', () => {
338331
})
339332

340333
it('should be able to filter to starred experiments', async () => {
341-
const { experiments, messageSpy } = buildExperiments({
342-
disposer: disposable
343-
})
344-
334+
const { experiments, messageSpy } =
335+
stubWorkspaceExperimentsGetters(disposable)
345336
await experiments.isReady()
346337
await experiments.showWebview()
347338

348-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
349-
350339
await addFilterViaQuickInput(experiments, starredFilter)
351340

352341
const [workspace, main, fe2919b, _7df876c] = rowsFixture
@@ -373,14 +362,10 @@ suite('Experiments Filter By Tree Test Suite', () => {
373362
}).timeout(WEBVIEW_TEST_TIMEOUT)
374363

375364
it('should provide a shortcut to filter to starred experiments', async () => {
376-
const { experiments, experimentsModel } = buildExperiments({
377-
disposer: disposable
378-
})
379-
365+
const { experiments, experimentsModel } =
366+
stubWorkspaceExperimentsGetters(disposable)
380367
await experiments.isReady()
381368

382-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
383-
384369
const mockAddFilter = stub(experimentsModel, 'addFilter')
385370

386371
await commands.executeCommand(

‎extension/src/test/suite/experiments/model/sortBy/tree.test.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,10 @@ suite('Experiments Sort By Tree Test Suite', () => {
164164
)
165165

166166
stub(WorkspaceExperiments.prototype, 'getDvcRoots').returns([dvcDemoPath])
167-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
167+
stub(WorkspaceExperiments.prototype, 'getOnlyOrPickProject').resolves(
168+
dvcDemoPath
169+
)
170+
stub(WorkspaceExperiments.prototype, 'getRepository').returns(experiments)
168171

169172
mockSortQuickPicks(testParamPath, false)
170173
const tableChangedPromise = experimentsUpdatedEvent(experiments)
@@ -284,14 +287,10 @@ suite('Experiments Sort By Tree Test Suite', () => {
284287
}).timeout(WEBVIEW_TEST_TIMEOUT)
285288

286289
it('should provide a shortcut to sort by starred experiments', async () => {
287-
const { experiments, experimentsModel } = buildExperiments({
288-
disposer: disposable
289-
})
290-
290+
const { experiments, experimentsModel } =
291+
stubWorkspaceExperimentsGetters(disposable)
291292
await experiments.isReady()
292293

293-
stubWorkspaceExperimentsGetters(dvcDemoPath, experiments)
294-
295294
const mockAddSort = stub(experimentsModel, 'addSort')
296295

297296
await commands.executeCommand(

‎extension/src/test/suite/experiments/model/tree.test.ts‎

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -573,21 +573,19 @@ suite('Experiments Tree Test Suite', () => {
573573
})
574574

575575
it('should be able to queue an experiment from an existing one with dvc.views.experiments.queueExperiment', async () => {
576-
const { dvcExecutor, experiments, experimentsModel } = buildExperiments({
577-
disposer: disposable
578-
})
576+
const {
577+
dvcExecutor,
578+
experiments,
579+
experimentsModel,
580+
mockGetOnlyOrPickProject
581+
} = stubWorkspaceExperimentsGetters(disposable)
579582

580583
await experiments.isReady()
581584

582585
const mockExperimentRunQueue = stub(dvcExecutor, 'expRunQueue').resolves(
583586
'true'
584587
)
585588

586-
const [mockGetOnlyOrPickProject] = stubWorkspaceExperimentsGetters(
587-
'',
588-
experiments
589-
)
590-
591589
const getParamsSpy = spy(experimentsModel, 'getWorkspaceParams')
592590

593591
const mockShowQuickPick = stub(window, 'showQuickPick') as SinonStub<
@@ -632,21 +630,19 @@ suite('Experiments Tree Test Suite', () => {
632630
})
633631

634632
it('should be able to run a new experiment from an existing one with dvc.views.experiments.runExperiment', async () => {
635-
const { dvcRunner, experiments, experimentsModel } = buildExperiments({
636-
disposer: disposable
637-
})
633+
const {
634+
dvcRunner,
635+
experiments,
636+
experimentsModel,
637+
mockGetOnlyOrPickProject
638+
} = stubWorkspaceExperimentsGetters(disposable)
638639

639640
await experiments.isReady()
640641

641642
const mockRunExperiment = stub(dvcRunner, 'runExperiment').resolves(
642643
undefined
643644
)
644645

645-
const [mockGetOnlyOrPickProject] = stubWorkspaceExperimentsGetters(
646-
'',
647-
experiments
648-
)
649-
650646
const getParamsSpy = spy(experimentsModel, 'getWorkspaceParams')
651647

652648
const mockShowQuickPick = stub(window, 'showQuickPick') as SinonStub<
@@ -688,22 +684,19 @@ suite('Experiments Tree Test Suite', () => {
688684
})
689685

690686
it('should be able to reset and run a new checkpoint experiment from an existing one with dvc.views.experiments.resetAndRunCheckpointExperiment', async () => {
691-
const { dvcRunner, experiments, experimentsModel } = buildExperiments({
692-
disposer: disposable
693-
})
694-
687+
const {
688+
dvcRunner,
689+
experiments,
690+
experimentsModel,
691+
mockGetOnlyOrPickProject
692+
} = stubWorkspaceExperimentsGetters(disposable)
695693
await experiments.isReady()
696694

697695
const mockRunExperimentReset = stub(
698696
dvcRunner,
699697
'runExperimentReset'
700698
).resolves(undefined)
701699

702-
const [mockGetOnlyOrPickProject] = stubWorkspaceExperimentsGetters(
703-
'',
704-
experiments
705-
)
706-
707700
const getParamsSpy = spy(experimentsModel, 'getWorkspaceParams')
708701

709702
const mockShowQuickPick = stub(window, 'showQuickPick') as SinonStub<

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -244,9 +244,12 @@ export const buildExperimentsData = (
244244
}
245245

246246
export const stubWorkspaceExperimentsGetters = (
247-
dvcRoot: string,
248-
experiments: Experiments
247+
disposer: Disposer,
248+
dvcRoot = dvcDemoPath
249249
) => {
250+
const { dvcExecutor, dvcRunner, experiments, experimentsModel, messageSpy } =
251+
buildExperiments({ disposer })
252+
250253
const mockGetOnlyOrPickProject = stub(
251254
WorkspaceExperiments.prototype,
252255
'getOnlyOrPickProject'
@@ -257,5 +260,13 @@ export const stubWorkspaceExperimentsGetters = (
257260
'getRepository'
258261
).returns(experiments)
259262

260-
return [mockGetOnlyOrPickProject, mockGetRepository]
263+
return {
264+
dvcExecutor,
265+
dvcRunner,
266+
experiments,
267+
experimentsModel,
268+
messageSpy,
269+
mockGetOnlyOrPickProject,
270+
mockGetRepository
271+
}
261272
}

0 commit comments

Comments
 (0)