Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only patch workspace only calls to plots diff #2629

Merged
merged 4 commits into from
Oct 19, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
remove get default revisions method
mattseddon committed Oct 19, 2022
commit 7dada3803a22d9c942d109d46fdbe8f6e0427809
5 changes: 4 additions & 1 deletion extension/src/experiments/model/index.ts
Original file line number Diff line number Diff line change
@@ -223,7 +223,10 @@ export class ExperimentsModel extends ModelWithPersistence {
}

public getBranchRevisions() {
return this.branches.map(({ id, sha }) => ({ id, sha }))
return this.branches.map(({ id, sha }) => ({
id,
sha
}))
}

public getRevisions() {
2 changes: 1 addition & 1 deletion extension/src/plots/data/index.ts
Original file line number Diff line number Diff line change
@@ -77,7 +77,7 @@ export class PlotsData extends BaseData<{ data: PlotsOutput; revs: string[] }> {
this.model &&
(sameContents(revs, ['workspace']) || sameContents(revs, []))
) {
return this.model.getDefaultRevs()
return []
}

return revs
4 changes: 0 additions & 4 deletions extension/src/plots/model/index.ts
Original file line number Diff line number Diff line change
@@ -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
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
@@ -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)
@@ -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
@@ -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 () => {