Skip to content

Commit 587aa5b

Browse files
authored
Prevent unnecessary CLI calls in the integration test suite (#2934)
* prevent unnecessary CLI calls in the integration test suite * flush sneaky extra watchers
1 parent 211cf59 commit 587aa5b

File tree

19 files changed

+84
-104
lines changed

19 files changed

+84
-104
lines changed

extension/src/experiments/data/index.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,6 @@ export class ExperimentsData extends BaseData<ExperimentsOutput> {
6666
return this.notifyChanged(data)
6767
}
6868

69-
public forceUpdate() {
70-
return this.processManager.forceRunQueued()
71-
}
72-
7369
protected collectFiles(data: ExperimentsOutput) {
7470
this.collectedFiles = collectFiles(data, this.collectedFiles)
7571
}

extension/src/experiments/index.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,6 @@ export class Experiments extends BaseRepository<TableData> {
163163
return this.cliData.managedUpdate()
164164
}
165165

166-
public forceUpdate() {
167-
return this.cliData.forceUpdate()
168-
}
169-
170166
public async setState(data: ExperimentsOutput) {
171167
const dvcLiveOnly = await checkSignalFile(
172168
join(this.dvcRoot, DVCLIVE_ONLY_RUNNING_SIGNAL_FILE)

extension/src/processManager.ts

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,6 @@ export class ProcessManager extends Disposable {
5555
return this.runQueued()
5656
}
5757

58-
public async forceRunQueued(): Promise<void> {
59-
const next = this.getNextFromQueue()
60-
if (!next) {
61-
return
62-
}
63-
64-
const { process } = this.processes[next]
65-
await Promise.all([process(), this.setLastStarted(next)])
66-
return this.forceRunQueued()
67-
}
68-
6958
public isOngoingOrQueued(name: string) {
7059
return this.isOngoing(name) || this.isQueued(name)
7160
}
@@ -92,7 +81,7 @@ export class ProcessManager extends Disposable {
9281

9382
private runQueued(): Promise<void> {
9483
const next = this.getNextFromQueue()
95-
if (!next) {
84+
if (!next || this.dispose.disposed) {
9685
return Promise.resolve()
9786
}
9887

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ import { afterEach, beforeEach, describe, it, suite } from 'mocha'
22
import { EventEmitter, RelativePattern } from 'vscode'
33
import { expect } from 'chai'
44
import { stub, restore, spy } from 'sinon'
5-
import { Disposable } from '../../../../extension'
65
import {
76
bypassProcessManagerDebounce,
87
getFirstArgOfCall,
98
getMockNow,
9+
getSafeWatcherDisposer,
1010
stubPrivateMemberMethod
1111
} from '../../util'
1212
import { dvcDemoPath, getTestWorkspaceFolder } from '../../../util'
@@ -27,14 +27,14 @@ import { gitPath } from '../../../../cli/git/constants'
2727
import { getGitPath } from '../../../../fileSystem'
2828

2929
suite('Experiments Data Test Suite', () => {
30-
const disposable = Disposable.fn()
30+
const disposable = getSafeWatcherDisposer()
3131

3232
beforeEach(() => {
3333
restore()
3434
})
3535

3636
afterEach(() => {
37-
disposable.dispose()
37+
return disposable.disposeAndFlush()
3838
})
3939

4040
describe('ExperimentsData', () => {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ import {
1212
window
1313
} from 'vscode'
1414
import { addFilterViaQuickInput } from './filterBy/util'
15-
import { Disposable } from '../../../../extension'
1615
import { ExperimentsModel, ExperimentType } from '../../../../experiments/model'
1716
import { UNSELECTED } from '../../../../experiments/model/status'
1817
import {
1918
experimentsUpdatedEvent,
2019
getFirstArgOfLastCall,
20+
getSafeWatcherDisposer,
2121
spyOnPrivateMethod,
2222
stubPrivatePrototypeMethod
2323
} from '../../util'
@@ -55,14 +55,14 @@ import { ExperimentItem } from '../../../../experiments/model/collect'
5555
import { EXPERIMENT_WORKSPACE_ID } from '../../../../cli/dvc/contract'
5656

5757
suite('Experiments Tree Test Suite', () => {
58-
const disposable = Disposable.fn()
58+
const disposable = getSafeWatcherDisposer()
5959

6060
beforeEach(() => {
6161
restore()
6262
})
6363

6464
afterEach(() => {
65-
disposable.dispose()
65+
return disposable.disposeAndFlush()
6666
})
6767

6868
// eslint-disable-next-line sonarjs/cognitive-complexity

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ import {
1010
buildDependencies,
1111
buildInternalCommands,
1212
buildMockData,
13-
mockDisposable
13+
mockDisposable,
14+
SafeWatcherDisposer
1415
} from '../util'
1516
import {
1617
ExperimentsOutput,
@@ -95,7 +96,7 @@ export const buildExperiments = (
9596
}
9697
}
9798

98-
export const buildMultiRepoExperiments = (disposer: Disposer) => {
99+
export const buildMultiRepoExperiments = (disposer: SafeWatcherDisposer) => {
99100
const {
100101
internalCommands,
101102
experiments: mockExperiments,
@@ -126,7 +127,7 @@ export const buildMultiRepoExperiments = (disposer: Disposer) => {
126127
return { experiments, internalCommands, messageSpy, workspaceExperiments }
127128
}
128129

129-
export const buildSingleRepoExperiments = (disposer: Disposer) => {
130+
export const buildSingleRepoExperiments = (disposer: SafeWatcherDisposer) => {
130131
const {
131132
internalCommands,
132133
gitReader,
@@ -153,6 +154,7 @@ export const buildSingleRepoExperiments = (disposer: Disposer) => {
153154

154155
return { messageSpy, workspaceExperiments }
155156
}
157+
156158
const buildExperimentsDataDependencies = (disposer: Disposer) => {
157159
const mockCreateFileSystemWatcher = stub(
158160
Watcher,
@@ -164,7 +166,7 @@ const buildExperimentsDataDependencies = (disposer: Disposer) => {
164166
return { internalCommands, mockCreateFileSystemWatcher, mockExperimentShow }
165167
}
166168

167-
export const buildExperimentsData = (disposer: Disposer) => {
169+
export const buildExperimentsData = (disposer: SafeWatcherDisposer) => {
168170
const { internalCommands, mockExperimentShow, mockCreateFileSystemWatcher } =
169171
buildExperimentsDataDependencies(disposer)
170172

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,12 @@ import { Disposable } from '../../../extension'
1212
import { Experiments } from '../../../experiments'
1313
import * as QuickPick from '../../../vscode/quickPick'
1414
import { DvcExecutor } from '../../../cli/dvc/executor'
15-
import { closeAllEditors, getInputBoxEvent, mockDuration } from '../util'
15+
import {
16+
closeAllEditors,
17+
getInputBoxEvent,
18+
getSafeWatcherDisposer,
19+
mockDuration
20+
} from '../util'
1621
import { dvcDemoPath } from '../../util'
1722
import { RegisteredCliCommands } from '../../../commands/external'
1823
import * as Telemetry from '../../../telemetry'
@@ -30,15 +35,14 @@ import { GitExecutor } from '../../../cli/git/executor'
3035
import { EXPERIMENT_WORKSPACE_ID } from '../../../cli/dvc/contract'
3136

3237
suite('Workspace Experiments Test Suite', () => {
33-
const disposable = Disposable.fn()
38+
const disposable = getSafeWatcherDisposer()
3439

3540
beforeEach(() => {
3641
restore()
3742
})
3843

3944
afterEach(() => {
40-
disposable.dispose()
41-
return closeAllEditors()
45+
return Promise.all([closeAllEditors(), disposable.disposeAndFlush()])
4246
})
4347

4448
const onDidChangeIsWebviewFocused = (

extension/src/test/suite/extension.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { window, commands, workspace, Uri } from 'vscode'
66
import {
77
closeAllEditors,
88
configurationChangeEvent,
9+
mockDisposable,
910
mockDuration,
1011
quickPickInitialized,
1112
selectQuickPickItem
@@ -23,6 +24,7 @@ import {
2324
RegisteredCommands
2425
} from '../../commands/external'
2526
import * as Setup from '../../setup'
27+
import * as Watcher from '../../fileSystem/watcher'
2628
import * as Telemetry from '../../telemetry'
2729
import { EventName } from '../../telemetry/constants'
2830
import { OutputChannel } from '../../vscode/outputChannel'
@@ -447,6 +449,7 @@ suite('Extension Test Suite', () => {
447449
})
448450

449451
it('should set the dvc.cli.incompatible context value', async () => {
452+
stub(Watcher, 'createFileSystemWatcher').returns(mockDisposable)
450453
stub(DvcReader.prototype, 'expShow').resolves({
451454
[EXPERIMENT_WORKSPACE_ID]: { baseline: {} }
452455
})

extension/src/test/suite/fileSystem/data/index.test.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,26 @@ import { afterEach, beforeEach, describe, it, suite } from 'mocha'
22
import { stub, restore } from 'sinon'
33
import { expect } from 'chai'
44
import { RelativePattern } from 'vscode'
5-
import { Disposable } from '../../../../extension'
65
import { FileSystemData } from '../../../../fileSystem/data'
76
import { dvcDemoPath, getTestWorkspaceFolder } from '../../../util'
87
import * as FileSystem from '../../../../fileSystem'
98
import * as Watcher from '../../../../fileSystem/watcher'
10-
import { getFirstArgOfCall, mockDisposable } from '../../util'
9+
import {
10+
getFirstArgOfCall,
11+
getSafeWatcherDisposer,
12+
mockDisposable
13+
} from '../../util'
1114
import { join } from '../../../util/path'
1215

1316
suite('File System Data Test Suite', () => {
14-
const disposable = Disposable.fn()
17+
const disposable = getSafeWatcherDisposer()
1518

1619
beforeEach(() => {
1720
restore()
1821
})
1922

2023
afterEach(() => {
21-
disposable.dispose()
24+
return disposable.disposeAndFlush()
2225
})
2326

2427
describe('FileSystemData', () => {

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ suite('GetStarted Test Suite', () => {
2323

2424
describe('Get Started', () => {
2525
it('should handle a initialize project message from the webview', async () => {
26-
const { messageSpy, getStarted, mockInitializeProject } =
27-
await buildGetStarted(disposable, true)
26+
const { messageSpy, getStarted, mockInitializeProject } = buildGetStarted(
27+
disposable,
28+
true
29+
)
2830

2931
const webview = await getStarted.showWebview()
3032
await webview.isReady()
@@ -40,8 +42,10 @@ suite('GetStarted Test Suite', () => {
4042
}).timeout(WEBVIEW_TEST_TIMEOUT)
4143

4244
it('should handle a open experiments message from the webview', async () => {
43-
const { messageSpy, getStarted, mockOpenExperiments } =
44-
await buildGetStarted(disposable, true)
45+
const { messageSpy, getStarted, mockOpenExperiments } = buildGetStarted(
46+
disposable,
47+
true
48+
)
4549

4650
const webview = await getStarted.showWebview()
4751
await webview.isReady()
@@ -57,7 +61,7 @@ suite('GetStarted Test Suite', () => {
5761
}).timeout(WEBVIEW_TEST_TIMEOUT)
5862

5963
it('should log an error message if the message from the webview is anything else than initialize project', async () => {
60-
const { messageSpy, getStarted } = await buildGetStarted(disposable)
64+
const { messageSpy, getStarted } = buildGetStarted(disposable)
6165

6266
const webview = await getStarted.showWebview()
6367
await webview.isReady()

0 commit comments

Comments
 (0)