Skip to content

Commit ea3e963

Browse files
authored
Do not drop experiment selection when an experiment is running and exp show errors (#4042)
1 parent 802cf2a commit ea3e963

File tree

2 files changed

+122
-1
lines changed

2 files changed

+122
-1
lines changed

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

Lines changed: 118 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import { join } from 'path'
33
import { commands } from 'vscode'
44
import { ExperimentsModel } from '.'
5+
import { copyOriginalColors } from './status/colors'
56
import gitLogFixture from '../../test/fixtures/expShow/base/gitLog'
67
import rowOrderFixture from '../../test/fixtures/expShow/base/rowOrder'
78
import outputFixture from '../../test/fixtures/expShow/base/output'
@@ -20,7 +21,9 @@ import survivalRowsFixture from '../../test/fixtures/expShow/survival/rows'
2021
import {
2122
ExperimentStatus,
2223
EXPERIMENT_WORKSPACE_ID,
23-
Executor
24+
Executor,
25+
ExpWithError,
26+
ExpShowOutput
2427
} from '../../cli/dvc/contract'
2528
import { PersistenceKey } from '../../persistence/constants'
2629

@@ -40,6 +43,8 @@ const DEFAULT_DATA: [
4043
{ [branch: string]: number }
4144
] = ['', false, [], { main: 2000 }]
4245

46+
type TransformAndSetInputs = [ExpShowOutput, ...typeof DEFAULT_DATA]
47+
4348
describe('ExperimentsModel', () => {
4449
it('should return the expected rows when given the base fixture', () => {
4550
const model = new ExperimentsModel('', buildMockMemento())
@@ -405,4 +410,116 @@ describe('ExperimentsModel', () => {
405410
'C'
406411
])
407412
})
413+
414+
const getSelectedRevisions = (
415+
model: ExperimentsModel
416+
): { id: string; displayColor: string }[] =>
417+
model
418+
.getSelectedRevisions()
419+
.map(({ displayColor, id }) => ({ displayColor, id }))
420+
421+
it('should not update the status of experiments if exp show fails and there was a running experiment', () => {
422+
const memento = buildMockMemento()
423+
const model = new ExperimentsModel('', memento)
424+
const colorList = copyOriginalColors()
425+
const expectedSelection = [
426+
{ id: 'exp-83425', displayColor: colorList[0] },
427+
{ id: 'exp-e7a67', displayColor: colorList[1] },
428+
{ id: EXPERIMENT_WORKSPACE_ID, displayColor: colorList[2] }
429+
]
430+
431+
const runningExperimentData: TransformAndSetInputs = [
432+
outputFixture,
433+
gitLogFixture,
434+
false,
435+
[],
436+
{
437+
main: 2000
438+
}
439+
]
440+
441+
const transientErrorData: TransformAndSetInputs = [
442+
[
443+
{
444+
rev: EXPERIMENT_WORKSPACE_ID,
445+
error: {
446+
type: 'caught error',
447+
msg:
448+
'unexpected error - [Errno 2]' +
449+
"No such file or directory: '.dvc/tmp/exps/run/ee47660cc5723ec201b88aa0fb8002e47508ee65/ee47660cc5723ec201b88aa0fb8002e47508ee65.run'" +
450+
'Having any troubles? Hit us up at https://dvc.org/support, we are always happy to help!'
451+
}
452+
} as ExpWithError
453+
],
454+
gitLogFixture,
455+
false,
456+
[],
457+
{
458+
main: 2000
459+
}
460+
]
461+
462+
model.transformAndSet(...runningExperimentData)
463+
464+
model.toggleStatus('exp-e7a67')
465+
model.toggleStatus(EXPERIMENT_WORKSPACE_ID)
466+
467+
expect(getSelectedRevisions(model)).toStrictEqual(expectedSelection)
468+
expect(model.hasRunningExperiment()).toBe(true)
469+
470+
model.transformAndSet(...transientErrorData)
471+
472+
expect(getSelectedRevisions(model)).toStrictEqual(
473+
expectedSelection.slice(2)
474+
)
475+
expect(model.hasRunningExperiment()).toBe(true)
476+
477+
model.transformAndSet(...runningExperimentData)
478+
479+
expect(getSelectedRevisions(model)).toStrictEqual(expectedSelection)
480+
expect(model.hasRunningExperiment()).toBe(true)
481+
})
482+
483+
it('should update the status of experiments if exp show fails and there was not a running experiment', () => {
484+
const memento = buildMockMemento()
485+
const model = new ExperimentsModel('', memento)
486+
const colorList = copyOriginalColors()
487+
488+
const noRunningExperimentData: TransformAndSetInputs = [
489+
survivalOutputFixture,
490+
...DEFAULT_DATA
491+
]
492+
493+
const unexpectedErrorData: TransformAndSetInputs = [
494+
[
495+
{
496+
rev: EXPERIMENT_WORKSPACE_ID,
497+
error: {
498+
type: 'caught error',
499+
msg: 'unexpected - error'
500+
}
501+
} as ExpWithError
502+
],
503+
...DEFAULT_DATA
504+
]
505+
506+
model.transformAndSet(...noRunningExperimentData)
507+
508+
model.toggleStatus('main')
509+
510+
expect(getSelectedRevisions(model)).toStrictEqual([
511+
{ id: 'main', displayColor: colorList[0] }
512+
])
513+
expect(model.hasRunningExperiment()).toBe(false)
514+
515+
model.transformAndSet(...unexpectedErrorData)
516+
517+
expect(getSelectedRevisions(model)).toStrictEqual([])
518+
expect(model.hasRunningExperiment()).toBe(false)
519+
520+
model.transformAndSet(...noRunningExperimentData)
521+
522+
expect(getSelectedRevisions(model)).toStrictEqual([])
523+
expect(model.hasRunningExperiment()).toBe(false)
524+
})
408525
})

extension/src/experiments/model/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,10 @@ export class ExperimentsModel extends ModelWithPersistence {
145145
this.experimentsByCommit = experimentsByCommit
146146
this.checkpoints = hasCheckpoints
147147

148+
const isTransientError = this.hasRunningExperiment() && workspace.error
149+
if (isTransientError) {
150+
return
151+
}
148152
this.setColoredStatus(runningExperiments)
149153
}
150154

0 commit comments

Comments
 (0)