From 1a8602346317cc461e9664bb0b66db1df5430e88 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Tue, 6 Jun 2023 11:36:37 +1000 Subject: [PATCH] Do not drop experiment selection when an experiment is running and exp show errors --- extension/src/experiments/model/index.test.ts | 119 +++++++++++++++++- extension/src/experiments/model/index.ts | 4 + 2 files changed, 122 insertions(+), 1 deletion(-) diff --git a/extension/src/experiments/model/index.test.ts b/extension/src/experiments/model/index.test.ts index 0f2d5019d2..59a987b4e3 100644 --- a/extension/src/experiments/model/index.test.ts +++ b/extension/src/experiments/model/index.test.ts @@ -2,6 +2,7 @@ import { join } from 'path' import { commands } from 'vscode' import { ExperimentsModel } from '.' +import { copyOriginalColors } from './status/colors' import gitLogFixture from '../../test/fixtures/expShow/base/gitLog' import rowOrderFixture from '../../test/fixtures/expShow/base/rowOrder' import outputFixture from '../../test/fixtures/expShow/base/output' @@ -20,7 +21,9 @@ import survivalRowsFixture from '../../test/fixtures/expShow/survival/rows' import { ExperimentStatus, EXPERIMENT_WORKSPACE_ID, - Executor + Executor, + ExpWithError, + ExpShowOutput } from '../../cli/dvc/contract' import { PersistenceKey } from '../../persistence/constants' @@ -40,6 +43,8 @@ const DEFAULT_DATA: [ { [branch: string]: number } ] = ['', false, [], { main: 2000 }] +type TransformAndSetInputs = [ExpShowOutput, ...typeof DEFAULT_DATA] + describe('ExperimentsModel', () => { it('should return the expected rows when given the base fixture', () => { const model = new ExperimentsModel('', buildMockMemento()) @@ -405,4 +410,116 @@ describe('ExperimentsModel', () => { 'C' ]) }) + + const getSelectedRevisions = ( + model: ExperimentsModel + ): { id: string; displayColor: string }[] => + model + .getSelectedRevisions() + .map(({ displayColor, id }) => ({ displayColor, id })) + + it('should not update the status of experiments if exp show fails and there was a running experiment', () => { + const memento = buildMockMemento() + const model = new ExperimentsModel('', memento) + const colorList = copyOriginalColors() + const expectedSelection = [ + { id: 'exp-83425', displayColor: colorList[0] }, + { id: 'exp-e7a67', displayColor: colorList[1] }, + { id: EXPERIMENT_WORKSPACE_ID, displayColor: colorList[2] } + ] + + const runningExperimentData: TransformAndSetInputs = [ + outputFixture, + gitLogFixture, + false, + [], + { + main: 2000 + } + ] + + const transientErrorData: TransformAndSetInputs = [ + [ + { + rev: EXPERIMENT_WORKSPACE_ID, + error: { + type: 'caught error', + msg: + 'unexpected error - [Errno 2]' + + "No such file or directory: '.dvc/tmp/exps/run/ee47660cc5723ec201b88aa0fb8002e47508ee65/ee47660cc5723ec201b88aa0fb8002e47508ee65.run'" + + 'Having any troubles? Hit us up at https://dvc.org/support, we are always happy to help!' + } + } as ExpWithError + ], + gitLogFixture, + false, + [], + { + main: 2000 + } + ] + + model.transformAndSet(...runningExperimentData) + + model.toggleStatus('exp-e7a67') + model.toggleStatus(EXPERIMENT_WORKSPACE_ID) + + expect(getSelectedRevisions(model)).toStrictEqual(expectedSelection) + expect(model.hasRunningExperiment()).toBe(true) + + model.transformAndSet(...transientErrorData) + + expect(getSelectedRevisions(model)).toStrictEqual( + expectedSelection.slice(2) + ) + expect(model.hasRunningExperiment()).toBe(true) + + model.transformAndSet(...runningExperimentData) + + expect(getSelectedRevisions(model)).toStrictEqual(expectedSelection) + expect(model.hasRunningExperiment()).toBe(true) + }) + + it('should update the status of experiments if exp show fails and there was not a running experiment', () => { + const memento = buildMockMemento() + const model = new ExperimentsModel('', memento) + const colorList = copyOriginalColors() + + const noRunningExperimentData: TransformAndSetInputs = [ + survivalOutputFixture, + ...DEFAULT_DATA + ] + + const unexpectedErrorData: TransformAndSetInputs = [ + [ + { + rev: EXPERIMENT_WORKSPACE_ID, + error: { + type: 'caught error', + msg: 'unexpected - error' + } + } as ExpWithError + ], + ...DEFAULT_DATA + ] + + model.transformAndSet(...noRunningExperimentData) + + model.toggleStatus('main') + + expect(getSelectedRevisions(model)).toStrictEqual([ + { id: 'main', displayColor: colorList[0] } + ]) + expect(model.hasRunningExperiment()).toBe(false) + + model.transformAndSet(...unexpectedErrorData) + + expect(getSelectedRevisions(model)).toStrictEqual([]) + expect(model.hasRunningExperiment()).toBe(false) + + model.transformAndSet(...noRunningExperimentData) + + expect(getSelectedRevisions(model)).toStrictEqual([]) + expect(model.hasRunningExperiment()).toBe(false) + }) }) diff --git a/extension/src/experiments/model/index.ts b/extension/src/experiments/model/index.ts index c7d599b33a..4e5a917d85 100644 --- a/extension/src/experiments/model/index.ts +++ b/extension/src/experiments/model/index.ts @@ -145,6 +145,10 @@ export class ExperimentsModel extends ModelWithPersistence { this.experimentsByCommit = experimentsByCommit this.checkpoints = hasCheckpoints + const isTransientError = this.hasRunningExperiment() && workspace.error + if (isTransientError) { + return + } this.setColoredStatus(runningExperiments) }