Skip to content

Commit 1047e7a

Browse files
sroy3sroy3
authored
Call exp show with multiple branches (#3827)
* Call exp show with multiple branches * Fix workspace experiment branch * fix lint * Fix tests * Fix subrows * Add tests * Self review * Apply review comments * Read commits details for all branches * Change current branch to main in fixtures and tests * Fix tests
1 parent 807b83d commit 1047e7a

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+574
-474
lines changed

‎extension/src/cli/dvc/constants.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,11 @@ export enum QueueSubCommand {
8080
}
8181

8282
export enum ExperimentFlag {
83-
ALL_BRANCHES = '-a',
8483
NO_FETCH = '--no-fetch',
8584
QUEUE = '--queue',
8685
RESET = '--reset',
87-
NUM_COMMIT = '-n'
86+
NUM_COMMIT = '-n',
87+
REV = '--rev'
8888
}
8989

9090
export enum GcPreserveFlag {

‎extension/src/cli/dvc/contract.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,11 +90,13 @@ export type ExecutorState = {
9090
export type ExpWithError = {
9191
rev: string
9292
name?: string
93+
branch: string | undefined
9394
} & DvcError
9495

9596
type ExpWithData = {
9697
rev: string
9798
name?: string
99+
branch: string | undefined
98100
data: ExpData
99101
}
100102

‎extension/src/cli/dvc/reader.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ describe('CliReader', () => {
9797
const cliOutput = await dvcReader.expShow(cwd)
9898
expect(cliOutput).toStrictEqual([
9999
{
100+
branch: undefined,
100101
error: { msg: unexpectedStderr, type: 'Caught error' },
101102
rev: EXPERIMENT_WORKSPACE_ID
102103
}
@@ -110,7 +111,9 @@ describe('CliReader', () => {
110111
)
111112

112113
const cliOutput = await dvcReader.expShow(cwd)
113-
expect(cliOutput).toStrictEqual([{ rev: EXPERIMENT_WORKSPACE_ID }])
114+
expect(cliOutput).toStrictEqual([
115+
{ branch: undefined, rev: EXPERIMENT_WORKSPACE_ID }
116+
])
114117
})
115118
})
116119

‎extension/src/cli/dvc/reader.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,13 @@ export class DvcReader extends DvcCli {
7474
Flag.JSON
7575
)
7676
if (isDvcError(output) || isEmpty(output)) {
77-
return [{ rev: EXPERIMENT_WORKSPACE_ID, ...(output as DvcError) }]
77+
return [
78+
{
79+
branch: undefined,
80+
rev: EXPERIMENT_WORKSPACE_ID,
81+
...(output as DvcError)
82+
}
83+
]
7884
}
7985
return output
8086
}

‎extension/src/cli/git/constants.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@ export enum Command {
2525

2626
export enum Flag {
2727
ALL = '--all',
28+
COUNT = '--count',
2829
DIRECTORIES = '-d',
2930
DIRECTORY = '--directory',
3031
DOT = '.',
3132
EXCLUDE_STANDARD = '--exclude-standard',
3233
FORCE = '-f',
33-
FULL_HISTORY = '--full-history',
3434
GET_URL = '--get-url',
3535
HARD = '--hard',
3636
NAME_ONLY = '--name-only',

‎extension/src/cli/git/reader.test.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,4 +68,59 @@ describe('GitReader', () => {
6868
expect(cliOutput).toStrictEqual([])
6969
})
7070
})
71+
72+
describe('getCurrentBranch', () => {
73+
it('should match the expected output', async () => {
74+
const cwd = __dirname
75+
const branches = ['* main', 'exp-12', 'fix-bug-11', 'other']
76+
mockedCreateProcess.mockReturnValueOnce(
77+
getMockedProcess(branches.join('\n'))
78+
)
79+
80+
const cliOutput = await gitReader.getCurrentBranch(cwd)
81+
expect(cliOutput).toStrictEqual('main')
82+
expect(mockedCreateProcess).toHaveBeenCalledWith({
83+
args: ['branch'],
84+
cwd,
85+
executable: 'git'
86+
})
87+
})
88+
89+
it('should match the expected output for detached HEAD', async () => {
90+
const cwd = __dirname
91+
const branches = [
92+
'* (HEAD detached at 4d06da1b)',
93+
'main',
94+
'fix-bug-11',
95+
'other'
96+
]
97+
mockedCreateProcess.mockReturnValueOnce(
98+
getMockedProcess(branches.join('\n'))
99+
)
100+
101+
const cliOutput = await gitReader.getCurrentBranch(cwd)
102+
expect(cliOutput).toStrictEqual('4d06da1b')
103+
})
104+
105+
it('should return an empty string if the current branch cannot be found', async () => {
106+
const cwd = __dirname
107+
const branches = ['main', 'fix-bug-11', 'other']
108+
mockedCreateProcess.mockReturnValueOnce(
109+
getMockedProcess(branches.join('\n'))
110+
)
111+
112+
const cliOutput = await gitReader.getCurrentBranch(cwd)
113+
expect(cliOutput).toStrictEqual('')
114+
})
115+
116+
it('should return an empty string if the cli returns any type of error', async () => {
117+
const cwd = __dirname
118+
mockedCreateProcess.mockImplementationOnce(() => {
119+
throw new Error('unexpected error - something something')
120+
})
121+
122+
const cliOutput = await gitReader.getCurrentBranch(cwd)
123+
expect(cliOutput).toStrictEqual('')
124+
})
125+
})
71126
})

‎extension/src/cli/git/reader.ts‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@ import { GitCli } from '.'
33
import { Command, Flag } from './constants'
44
import { getOptions } from './options'
55
import { typeCheckCommands } from '..'
6-
import { trimAndSplit } from '../../util/stdout'
6+
import { cleanUpBranchName, trimAndSplit } from '../../util/stdout'
77
import { isDirectory } from '../../fileSystem'
88

99
export const autoRegisteredCommands = {
1010
GIT_GET_BRANCHES: 'getBranches',
1111
GIT_GET_COMMIT_MESSAGES: 'getCommitMessages',
12+
GIT_GET_CURRENT_BRANCH: 'getCurrentBranch',
1213
GIT_GET_NUM_COMMITS: 'getNumCommits',
1314
GIT_GET_REMOTE_URL: 'getRemoteUrl',
1415
GIT_GET_REPOSITORY_ROOT: 'getGitRepositoryRoot',
@@ -52,9 +53,11 @@ export class GitReader extends GitCli {
5253
const options = getOptions(
5354
cwd,
5455
Command.LOG,
55-
`${sha}^..HEAD`,
56+
sha,
5657
Flag.PRETTY_FORMAT_COMMIT_MESSAGE,
57-
Flag.SEPARATE_WITH_NULL
58+
Flag.SEPARATE_WITH_NULL,
59+
Flag.NUMBER,
60+
'1'
5861
)
5962
try {
6063
return await this.executeProcess(options)
@@ -80,31 +83,39 @@ export class GitReader extends GitCli {
8083
return new Set([...files, ...dirs])
8184
}
8285

83-
public async getNumCommits(cwd: string) {
84-
const options = getOptions(
85-
cwd,
86-
Command.REV_LIST,
87-
Flag.FULL_HISTORY,
88-
Flag.ALL
89-
)
86+
public async getNumCommits(cwd: string, branch: string) {
87+
const options = getOptions(cwd, Command.REV_LIST, Flag.COUNT, branch)
9088
try {
91-
const revisions = await this.executeProcess(options)
92-
return trimAndSplit(revisions).length
89+
const nbCommits = await this.executeProcess(options)
90+
return Number.parseInt(nbCommits)
9391
} catch {
94-
return ''
92+
return 0
9593
}
9694
}
9795

9896
public async getBranches(cwd: string): Promise<string[]> {
9997
const options = getOptions(cwd, Command.BRANCH, Flag.NO_MERGE)
10098
try {
10199
const branches = await this.executeProcess(options)
102-
return trimAndSplit(branches)
100+
return trimAndSplit(branches).map(cleanUpBranchName)
103101
} catch {
104102
return []
105103
}
106104
}
107105

106+
public async getCurrentBranch(cwd: string): Promise<string> {
107+
const options = getOptions(cwd, Command.BRANCH)
108+
try {
109+
const branches = await this.executeProcess(options)
110+
const currentBranch = trimAndSplit(branches).find(
111+
branch => branch.indexOf('*') === 0
112+
)
113+
return (currentBranch && cleanUpBranchName(currentBranch)) || ''
114+
} catch {
115+
return ''
116+
}
117+
}
118+
108119
private async getUntrackedDirectories(cwd: string): Promise<string[]> {
109120
const options = getOptions(
110121
cwd,

‎extension/src/experiments/columns/collect/index.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,7 @@ describe('collectRelativeMetricsFiles', () => {
733733
expect(
734734
collectRelativeMetricsFiles([
735735
{
736+
branch: 'main',
736737
rev: EXPERIMENT_WORKSPACE_ID,
737738
error: { msg: 'I broke', type: 'not important' }
738739
}

‎extension/src/experiments/data/collect.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ describe('collectFiles', () => {
1616
it('should handle an error being returned', () => {
1717
const workspaceOnly = [
1818
{
19+
branch: 'main',
1920
error: { msg: 'bad things are happening', type: 'today' },
2021
rev: EXPERIMENT_WORKSPACE_ID
2122
}

‎extension/src/experiments/data/index.ts‎

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
import { getRelativePattern } from '../../fileSystem/relativePattern'
88
import { createFileSystemWatcher } from '../../fileSystem/watcher'
99
import { AvailableCommands, InternalCommands } from '../../commands/internal'
10-
import { ExpShowOutput } from '../../cli/dvc/contract'
10+
import { EXPERIMENT_WORKSPACE_ID, ExpShowOutput } from '../../cli/dvc/contract'
1111
import { BaseData } from '../../data'
1212
import { DOT_DVC, ExperimentFlag } from '../../cli/dvc/constants'
1313
import { gitPath } from '../../cli/git/constants'
@@ -42,16 +42,32 @@ export class ExperimentsData extends BaseData<ExpShowOutput> {
4242

4343
public async update(): Promise<void> {
4444
void this.updateAvailableBranchesToSelect()
45-
const flags = this.experiments.getIsBranchesView()
46-
? [ExperimentFlag.ALL_BRANCHES]
47-
: [
45+
const data: ExpShowOutput = []
46+
47+
const { branches, currentBranch } =
48+
await this.getBranchesToShowWithCurrent()
49+
50+
await Promise.all(
51+
branches.map(async branch => {
52+
const branchFlags = [
53+
ExperimentFlag.REV,
54+
branch,
4855
ExperimentFlag.NUM_COMMIT,
49-
this.experiments.getNbOfCommitsToShow().toString()
56+
this.experiments.getNbOfCommitsToShow(branch).toString()
5057
]
51-
const data = await this.internalCommands.executeCommand<ExpShowOutput>(
52-
AvailableCommands.EXP_SHOW,
53-
this.dvcRoot,
54-
...flags
58+
const output = (await this.expShow(
59+
branchFlags,
60+
branch
61+
)) as ExpShowOutput
62+
63+
if (branch !== currentBranch) {
64+
const workspaceIndex = output.findIndex(
65+
exp => exp.rev === EXPERIMENT_WORKSPACE_ID
66+
)
67+
output.splice(workspaceIndex, 1)
68+
}
69+
data.push(...output)
70+
})
5571
)
5672

5773
this.collectFiles(data)
@@ -63,6 +79,30 @@ export class ExperimentsData extends BaseData<ExpShowOutput> {
6379
this.collectedFiles = collectFiles(data, this.collectedFiles)
6480
}
6581

82+
private async getBranchesToShowWithCurrent() {
83+
const currentBranch = await this.internalCommands.executeCommand<string>(
84+
AvailableCommands.GIT_GET_CURRENT_BRANCH,
85+
this.dvcRoot
86+
)
87+
88+
const branches = this.experiments.getBranchesToShow()
89+
90+
if (!branches.includes(currentBranch)) {
91+
branches.push(currentBranch)
92+
this.experiments.setBranchesToShow(branches)
93+
}
94+
return { branches, currentBranch }
95+
}
96+
97+
private async expShow(flags: (ExperimentFlag | string)[], branch?: string) {
98+
const data = await this.internalCommands.executeCommand<ExpShowOutput>(
99+
AvailableCommands.EXP_SHOW,
100+
this.dvcRoot,
101+
...flags
102+
)
103+
return data.map(exp => ({ ...exp, branch }))
104+
}
105+
66106
private async updateAvailableBranchesToSelect() {
67107
const allBranches = await this.internalCommands.executeCommand<string[]>(
68108
AvailableCommands.GIT_GET_BRANCHES,

0 commit comments

Comments
 (0)