STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Remove private readBranchConfig method and remove parseBranchConfig from Client

Browse source 
I think I went too far with my previous refactor and am backing out of it.
Adding a private readBranchConfig method on the client wasn't providing
any real additional value, so I've put it back into ReadBranchConfig.

However, I think there is still value in having parseBranchConfig
(formerly createBranchConfig) as a separate util function, as it both
improves readability of ReadBranchConfig and makes parsing its purpose
easier through the bespoke tests for it.
This commit is contained in:
Tyler McGoffin 2025-01-08 10:36:45 -08:00
parent 0137150848
commit 4575692ebf
2 changed files with 28 additions and 79 deletions
Download patch file Download diff file Expand all files Collapse all files

21
git/client.go
View file

@ -381,35 +381,26 @@ func (c *Client) lookupCommit(ctx context.Context, sha, format string) ([]byte,
// Downstream consumers of ReadBranchConfig should consider the behavior they desire if this errors,
// as an empty config is not necessarily breaking.
func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (BranchConfig, error) {
var cfg BranchConfig
branchConfigOutput, err := c.readGitBranchConfig(ctx, branch)
if err != nil {
return cfg, err
}
return c.createBranchConfig(branchConfigOutput), nil
}
func (c *Client) readGitBranchConfig(ctx context.Context, branch string) ([]string, error) {
prefix := regexp.QuoteMeta(fmt.Sprintf("branch.%s.", branch))
args := []string{"config", "--get-regexp", fmt.Sprintf("^%s(remote|merge|%s)$", prefix, MergeBaseConfig)}
cmd, err := c.Command(ctx, args...)
if err != nil {
return []string{}, err
return BranchConfig{}, err
}
out, err := cmd.Output()
if err != nil {
return []string{}, err
return BranchConfig{}, err
}
return outputLines(out), nil
return parseBranchConfig(outputLines(out)), nil
}
func (c *Client) createBranchConfig(gitBranchConfigOutput []string) BranchConfig {
func parseBranchConfig(configLines []string) BranchConfig {
var cfg BranchConfig
for _, line := range gitBranchConfigOutput {
for _, line := range configLines {
parts := strings.SplitN(line, " ", 2)
if len(parts) < 2 {
continue

86
git/client_test.go
View file

@ -730,14 +730,26 @@ func TestClientReadBranchConfig(t *testing.T) {
cmdExitStatus int
cmdStdout string
cmdStderr string
wantCmdArgs string
branch string
wantBranchConfig BranchConfig
wantError *GitError
}{
{
name: "read branch config",
cmdExitStatus: 0,
cmdStdout: "branch.trunk.remote origin\nbranch.trunk.merge refs/heads/trunk\nbranch.trunk.gh-merge-base trunk",
wantCmdArgs: `path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|gh-merge-base)$`,
branch: "trunk",
wantBranchConfig: BranchConfig{RemoteName: "origin", MergeRef: "refs/heads/trunk", MergeBase: "trunk"},
wantError: nil,
},
{
name: "command error",
cmdExitStatus: 1,
cmdStdout: "",
cmdStderr: "git error message",
branch: "trunk",
wantBranchConfig: BranchConfig{},
wantError: &GitError{ExitCode: 1, Stderr: "git error message"},
},
}
for _, tt := range tests {
@ -747,72 +759,19 @@ func TestClientReadBranchConfig(t *testing.T) {
GitPath: "path/to/git",
commandContext: cmdCtx,
}
branchConfig, _ := client.ReadBranchConfig(context.Background(), "trunk")
assert.Equal(t, tt.wantCmdArgs, strings.Join(cmd.Args[3:], " "))
branchConfig, err := client.ReadBranchConfig(context.Background(), tt.branch)
wantCmdArgs := fmt.Sprintf("path/to/git config --get-regexp ^branch\\.%s\\.(remote|merge|gh-merge-base)$", tt.branch)
assert.Equal(t, wantCmdArgs, strings.Join(cmd.Args[3:], " "))
assert.Equal(t, tt.wantBranchConfig, branchConfig)
})
}
}
func Test_readGitBranchConfig(t *testing.T) {
tests := []struct {
name string
branch string
cmdExitStatus int
cmdStdout string
cmdStderr string
expectedOutput []string
expectedError *GitError
}{
{
name: "read branch config",
branch: "trunk",
cmdExitStatus: 0,
cmdStdout: "branch.trunk.remote origin\nbranch.trunk.merge refs/heads/trunk\nbranch.trunk.gh-merge-base trunk",
expectedOutput: []string{
"branch.trunk.remote origin",
"branch.trunk.merge refs/heads/trunk",
"branch.trunk.gh-merge-base trunk"},
expectedError: nil,
},
{
name: "command failure",
branch: "trunk",
cmdExitStatus: 1,
cmdStderr: "git error message",
expectedOutput: []string{},
expectedError: &GitError{
ExitCode: 1,
Stderr: "git error message",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
commandString := fmt.Sprintf("path/to/git config --get-regexp ^branch\\.%s\\.(remote|merge|gh-merge-base)$", tt.branch)
cmdCtx := createMockedCommandContext(t, map[args]commandResult{
args(commandString): {
ExitStatus: tt.cmdExitStatus,
Stdout: tt.cmdStdout,
Stderr: tt.cmdStderr,
},
})
client := Client{
GitPath: "path/to/git",
commandContext: cmdCtx,
}
out, err := client.readGitBranchConfig(context.Background(), tt.branch)
assert.Equal(t, tt.expectedOutput, out)
if err != nil {
assert.Equal(t, tt.cmdStderr, err.(*GitError).Stderr)
assert.Equal(t, tt.cmdExitStatus, err.(*GitError).ExitCode)
if tt.wantError != nil {
assert.Equal(t, tt.wantError.ExitCode, err.(*GitError).ExitCode)
assert.Equal(t, tt.wantError.Stderr, err.(*GitError).Stderr)
}
})
}
}
func Test_createBranchConfig(t *testing.T) {
func Test_parseBranchConfig(t *testing.T) {
tests := []struct {
name string
gitBranchConfigOutput []string
@ -855,8 +814,7 @@ func Test_createBranchConfig(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
client := Client{}
branchConfig := client.createBranchConfig(tt.gitBranchConfigOutput)
branchConfig := parseBranchConfig(tt.gitBranchConfigOutput)
assert.Equal(t, tt.wantBranchConfig, branchConfig)
})
}