Skip to content

Commit

Permalink
Only patch workspace only calls to plots diff (#2629)
Browse files Browse the repository at this point in the history
* remove get default revisions method

* simplify if statement

* revert formatting change in experiments model
  • Loading branch information
mattseddon authored Oct 19, 2022
1 parent 658b1a9 commit 0fb7275
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 35 deletions.
8 changes: 3 additions & 5 deletions extension/src/plots/data/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,9 @@ export class PlotsData extends BaseData<{ data: PlotsOutput; revs: string[] }> {
}

private getArgs(revs: string[]) {
if (
this.model &&
(sameContents(revs, ['workspace']) || sameContents(revs, []))
) {
return this.model.getDefaultRevs()
const cliWillThrowError = sameContents(revs, ['workspace'])
if (this.model && cliWillThrowError) {
return []
}

return revs
Expand Down
4 changes: 0 additions & 4 deletions extension/src/plots/model/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,6 @@ export class PlotsModel extends ModelWithPersistence {
)
}

public getDefaultRevs() {
return ['workspace', ...Object.values(this.branchRevisions)]
}

public getTemplatePlots(order: TemplateOrder | undefined) {
if (!definedAndNonEmpty(order)) {
return
Expand Down
31 changes: 5 additions & 26 deletions extension/src/test/suite/plots/data/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ suite('Plots Data Test Suite', () => {
const buildPlotsData = (
experimentIsRunning: boolean,
missingRevisions: string[] = [],
mutableRevisions: string[] = [],
defaultRevs: string[] = []
mutableRevisions: string[] = []
) => {
const { internalCommands, updatesPaused, mockPlotsDiff, dvcRunner } =
buildDependencies(disposable)
Expand All @@ -36,10 +35,8 @@ suite('Plots Data Test Suite', () => {

const mockGetMissingRevisions = stub().returns(missingRevisions)
const mockGetMutableRevisions = stub().returns(mutableRevisions)
const mockGetDefaultRevs = stub().returns(defaultRevs)

const mockPlotsModel = {
getDefaultRevs: mockGetDefaultRevs,
getMissingRevisions: mockGetMissingRevisions,
getMutableRevisions: mockGetMutableRevisions
} as unknown as PlotsModel
Expand All @@ -62,38 +59,20 @@ suite('Plots Data Test Suite', () => {
})

it('should call plots diff when there are no revisions to fetch and no experiment is running (workspace updates)', async () => {
const defaultRevisions = ['workspace', '4d78b9e']
const { data, mockPlotsDiff } = buildPlotsData(
false,
[],
[],
defaultRevisions
)
const { data, mockPlotsDiff } = buildPlotsData(false, [], [])

await data.update()

expect(mockPlotsDiff).to.be.calledOnce
expect(mockPlotsDiff).to.be.calledWithExactly(
dvcDemoPath,
...defaultRevisions
)
expect(mockPlotsDiff).to.be.calledWithExactly(dvcDemoPath)
})

it('should call plots diff when an experiment is running in the workspace (live updates)', async () => {
const defaultRevisions = ['workspace', '4d78b9e']
const { data, mockPlotsDiff } = buildPlotsData(
true,
[],
['workspace'],
defaultRevisions
)
const { data, mockPlotsDiff } = buildPlotsData(true, [], ['workspace'])

await data.update()

expect(mockPlotsDiff).to.be.calledWithExactly(
dvcDemoPath,
...defaultRevisions
)
expect(mockPlotsDiff).to.be.calledWithExactly(dvcDemoPath)
})

it('should call plots diff when an experiment is running in a temporary directory (live updates)', async () => {
Expand Down

0 comments on commit 0fb7275

Please sign in to comment.