Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge branch 'main' into queue-quick-pick
Browse files Browse the repository at this point in the history
mattseddon authored Jan 4, 2023
2 parents 89e1efa + 14145cc commit 832a176
Showing 10 changed files with 338 additions and 160 deletions.
10 changes: 5 additions & 5 deletions extension/package.json
Original file line number Diff line number Diff line change
@@ -1604,10 +1604,10 @@
"@types/vscode": "1.64.0",
"@vscode/test-electron": "2.2.1",
"@vscode/vsce": "2.16.0",
"@wdio/cli": "8.0.13",
"@wdio/local-runner": "8.0.13",
"@wdio/mocha-framework": "8.0.13",
"@wdio/spec-reporter": "8.0.13",
"@wdio/cli": "8.0.15",
"@wdio/local-runner": "8.0.15",
"@wdio/mocha-framework": "8.0.14",
"@wdio/spec-reporter": "8.0.14",
"chai": "4.3.7",
"chai-as-promised": "7.1.1",
"clean-webpack-plugin": "4.0.0",
@@ -1625,7 +1625,7 @@
"ts-loader": "9.4.2",
"vscode-uri": "3.0.7",
"wdio-vscode-service": "5.0.0",
"webdriverio": "8.0.13",
"webdriverio": "8.0.15",
"webpack": "5.75.0",
"webpack-cli": "5.0.1"
},
30 changes: 26 additions & 4 deletions extension/src/experiments/index.ts
Original file line number Diff line number Diff line change
@@ -41,7 +41,7 @@ import { createTypedAccumulator } from '../util/object'
import { pickPaths } from '../path/selection/quickPick'
import { Toast } from '../vscode/toast'
import { ConfigKey } from '../vscode/config'
import { checkSignalFile } from '../fileSystem'
import { checkSignalFile, pollSignalFileForProcess } from '../fileSystem'
import { DVCLIVE_ONLY_RUNNING_SIGNAL_FILE } from '../cli/dvc/constants'

export const ExperimentsScale = {
@@ -94,6 +94,9 @@ export class Experiments extends BaseRepository<TableData> {
private readonly internalCommands: InternalCommands
private readonly webviewMessages: WebviewMessages

private dvcLiveOnlyCleanupInitialized = false
private dvcLiveOnlySignalFile: string

constructor(
dvcRoot: string,
internalCommands: InternalCommands,
@@ -105,6 +108,11 @@ export class Experiments extends BaseRepository<TableData> {
) {
super(dvcRoot, resourceLocator.beaker)

this.dvcLiveOnlySignalFile = join(
this.dvcRoot,
DVCLIVE_ONLY_RUNNING_SIGNAL_FILE
)

this.internalCommands = internalCommands

this.onDidChangeIsParamsFileFocused = this.paramsFileFocused.event
@@ -164,9 +172,8 @@ export class Experiments extends BaseRepository<TableData> {
}

public async setState(data: ExperimentsOutput) {
const dvcLiveOnly = await checkSignalFile(
join(this.dvcRoot, DVCLIVE_ONLY_RUNNING_SIGNAL_FILE)
)
const dvcLiveOnly = await this.checkSignalFile()

const commitMessages: { [sha: string]: string } =
await this.internalCommands.executeCommand(
AvailableCommands.GIT_GET_LAST_THREE_COMMIT_MESSAGES,
@@ -602,4 +609,19 @@ export class Experiments extends BaseRepository<TableData> {
this.onDidChangeColumns
)
}

private async checkSignalFile() {
const dvcLiveOnly = await checkSignalFile(this.dvcLiveOnlySignalFile)

if (dvcLiveOnly && !this.dvcLiveOnlyCleanupInitialized) {
this.dvcLiveOnlyCleanupInitialized = true
pollSignalFileForProcess(this.dvcLiveOnlySignalFile, () => {
this.dvcLiveOnlyCleanupInitialized = false
if (this.hasRunningExperiment()) {
this.cliData.update()
}
})
}
return dvcLiveOnly
}
}
14 changes: 14 additions & 0 deletions extension/src/fileSystem/index.ts
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@ import { createValidInteger } from '../util/number'
import { processExists } from '../processExecution'
import { getFirstWorkspaceFolder } from '../vscode/workspaceFolders'
import { DOT_DVC } from '../cli/dvc/constants'
import { delay } from '../util/time'

export const exists = (path: string): boolean => existsSync(path)

@@ -176,6 +177,19 @@ export const checkSignalFile = async (path: string): Promise<boolean> => {
return !!(await getPidFromSignalFile(path))
}

export const pollSignalFileForProcess = async (
path: string,
callback: () => void,
ms = 5000
): Promise<void> => {
await delay(ms)
const signalIsValid = await checkSignalFile(path)
if (signalIsValid) {
return pollSignalFileForProcess(path, callback, ms)
}
return callback()
}

export const getBinDisplayText = (
path: string | undefined
): string | undefined => {
21 changes: 8 additions & 13 deletions extension/src/test/suite/config.test.ts
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ import * as Extensions from '../../vscode/extensions'
import * as Python from '../../extensions/python'
import * as WorkspaceFolders from '../../vscode/workspaceFolders'
import { ConfigKey, setConfigValue } from '../../vscode/config'
import { dvcDemoPath } from '../util'

suite('Config Test Suite', () => {
const disposable = Disposable.fn()
@@ -76,15 +77,13 @@ suite('Config Test Suite', () => {
setupTriggeredCount = setupTriggeredCount + 1
})
)
const mockDvcMonoRepo = Uri.file(resolve('mono-repo')).fsPath
const mockDvcMonoRepo = Uri.file(resolve(dvcDemoPath)).fsPath
const mockGetWorkspaceFolders = stub(
WorkspaceFolders,
'getWorkspaceFolders'
).returns([mockDvcMonoRepo])
const mockDvcSubRoot1 = Uri.file(join(mockDvcMonoRepo, 'subroot1')).fsPath
const mockDvcSubRoot2 = Uri.file(
join(mockDvcMonoRepo, 'subroot2', 'deep', 'root')
).fsPath
const mockDvcSubRoot1 = Uri.file(join(mockDvcMonoRepo, 'data')).fsPath
const mockDvcSubRoot2 = Uri.file(join(mockDvcMonoRepo, '.dvc')).fsPath

await setConfigValue(ConfigKey.FOCUSED_PROJECTS, mockDvcSubRoot1)
await configUpdated
@@ -117,7 +116,7 @@ suite('Config Test Suite', () => {
expect(
config.getFocusedProjects(),
'should be able to focus multiple sub-projects'
).to.deep.equal([mockDvcSubRoot1, mockDvcSubRoot2])
).to.deep.equal([mockDvcSubRoot1, mockDvcSubRoot2].sort())
expect(setupTriggeredCount).to.equal(2)

await setConfigValue(ConfigKey.FOCUSED_PROJECTS, [
@@ -129,7 +128,7 @@ suite('Config Test Suite', () => {
expect(
config.getFocusedProjects(),
'should not call setup if the value(s) inside of the option have not changed'
).to.deep.equal([mockDvcSubRoot1, mockDvcSubRoot2])
).to.deep.equal([mockDvcSubRoot1, mockDvcSubRoot2].sort())
expect(setupTriggeredCount).to.equal(2)

await setConfigValue(ConfigKey.FOCUSED_PROJECTS, [
@@ -142,7 +141,7 @@ suite('Config Test Suite', () => {
expect(
config.getFocusedProjects(),
'should be able to focus multiple sub-projects along with the monorepo root'
).to.deep.equal([mockDvcMonoRepo, mockDvcSubRoot1, mockDvcSubRoot2])
).to.deep.equal([mockDvcMonoRepo, mockDvcSubRoot1, mockDvcSubRoot2].sort())
expect(setupTriggeredCount).to.equal(3)

await setConfigValue(ConfigKey.FOCUSED_PROJECTS, undefined)
@@ -165,11 +164,7 @@ suite('Config Test Suite', () => {

mockGetWorkspaceFolders.restore()

await setConfigValue(ConfigKey.FOCUSED_PROJECTS, [
mockDvcSubRoot2,
mockDvcSubRoot1,
mockDvcMonoRepo
])
await setConfigValue(ConfigKey.FOCUSED_PROJECTS, ['a', 'b', 'c'])
await configUpdated

expect(
52 changes: 52 additions & 0 deletions extension/src/test/suite/experiments/index.test.ts
Original file line number Diff line number Diff line change
@@ -71,6 +71,7 @@ import { PlotSizeNumber } from '../../../plots/webview/contract'
import { RegisteredCommands } from '../../../commands/external'
import { ConfigKey } from '../../../vscode/config'
import { EXPERIMENT_WORKSPACE_ID } from '../../../cli/dvc/contract'
import * as Time from '../../../util/time'

suite('Experiments Test Suite', () => {
const disposable = Disposable.fn()
@@ -1626,4 +1627,55 @@ suite('Experiments Test Suite', () => {
).to.deep.equal([])
})
})

describe('setState', () => {
it('should clean up after a killed DVCLive process that was running an experiment outside of the DVC context', async () => {
const defaultExperimentsData = { workspace: { baseline: { data: {} } } }

const { experiments, mockCheckSignalFile, mockUpdateExperimentsData } =
buildExperiments(disposable, defaultExperimentsData)

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const getCleanupInitialized = (experiments: any) =>
experiments.dvcLiveOnlyCleanupInitialized

await experiments.isReady()

const mockDelay = stub(Time, 'delay')
mockDelay.callsFake(() => mockDelay.wrappedMethod(500))

let processKilled = false

const cleanupUpdate = new Promise(resolve =>
mockUpdateExperimentsData.callsFake(() => resolve(undefined))
)

mockCheckSignalFile.resetBehavior()
mockCheckSignalFile.callsFake(() => {
return Promise.resolve(!processKilled)
})

const dataUpdated = new Promise(resolve =>
disposable.track(
experiments.onDidChangeExperiments(() => resolve(undefined))
)
)

experiments.setState(defaultExperimentsData)
await dataUpdated
expect(experiments.hasRunningExperiment()).to.be.true
expect(getCleanupInitialized(experiments)).to.be.true

processKilled = true

mockUpdateExperimentsData.resetHistory()

await cleanupUpdate

expect(getCleanupInitialized(experiments)).to.be.false
expect(mockCheckSignalFile).to.be.called
expect(mockDelay).to.be.called
expect(mockUpdateExperimentsData).to.be.calledOnce
})
})
})
8 changes: 7 additions & 1 deletion extension/src/test/suite/experiments/util.ts
Original file line number Diff line number Diff line change
@@ -60,14 +60,19 @@ export const buildExperiments = (
resourceLocator
} = buildDependencies(disposer, experimentShowData)

const mockUpdateExperimentsData = stub()
const mockExperimentsData = buildMockData<ExperimentsData>(
mockUpdateExperimentsData
)

const experiments = disposer.track(
new Experiments(
dvcRoot,
internalCommands,
updatesPaused,
resourceLocator,
buildMockMemento(),
buildMockData<ExperimentsData>(),
mockExperimentsData,
buildMockData<FileSystemData>()
)
)
@@ -90,6 +95,7 @@ export const buildExperiments = (
messageSpy,
mockCheckSignalFile,
mockExperimentShow,
mockUpdateExperimentsData,
resourceLocator,
updatesPaused
}
6 changes: 3 additions & 3 deletions extension/src/test/suite/setup/index.test.ts
Original file line number Diff line number Diff line change
@@ -191,7 +191,7 @@ suite('Setup Test Suite', () => {
const messageSent = new Promise(resolve =>
mockSendMessage.callsFake(data => {
resolve(undefined)
return mockSendMessage.wrappedMethod(data)
return Promise.resolve(!!data)
})
)

@@ -227,7 +227,7 @@ suite('Setup Test Suite', () => {
const messageSent = new Promise(resolve =>
mockSendMessage.callsFake(data => {
resolve(undefined)
return mockSendMessage.wrappedMethod(data)
return Promise.resolve(!!data)
})
)

@@ -268,7 +268,7 @@ suite('Setup Test Suite', () => {
const messageSent = new Promise(resolve =>
mockSendMessage.callsFake(data => {
resolve(undefined)
return mockSendMessage.wrappedMethod(data)
return Promise.resolve(!!data)
})
)

7 changes: 5 additions & 2 deletions extension/src/test/suite/util.ts
Original file line number Diff line number Diff line change
@@ -164,10 +164,13 @@ export const buildInternalCommands = (disposer: Disposer) => {
}
}

export const buildMockData = <T extends ExperimentsData | FileSystemData>() =>
export const buildMockData = <T extends ExperimentsData | FileSystemData>(
update = stub()
) =>
({
dispose: stub(),
onDidUpdate: stub()
onDidUpdate: stub(),
update
} as unknown as T)

export const buildDependencies = (
2 changes: 1 addition & 1 deletion webview/package.json
Original file line number Diff line number Diff line change
@@ -59,7 +59,7 @@
"@types/react": "18.0.26",
"@types/react-dom": "18.0.10",
"@types/react-measure": "2.0.8",
"@types/react-table": "7.7.12",
"@types/react-table": "7.7.13",
"@types/react-virtualized": "9.21.21",
"@types/webpack": "5.28.0",
"@welldone-software/why-did-you-render": "7.0.1",
348 changes: 217 additions & 131 deletions yarn.lock

Large diffs are not rendered by default.

0 comments on commit 832a176

Please sign in to comment.