fix: address code review — error-path tests, edge cases, UX improvements

This commit is contained in:
2026-06-10 18:20:54 +08:00
parent 493775cd1e
commit 8d16fabaed
6 changed files with 77 additions and 10 deletions
+1 -1
View File
@@ -52,7 +52,7 @@ func parseRemoteBranches(buf []byte) []string {
var branches []string var branches []string
sc := bufio.NewScanner(bytes.NewReader(buf)) sc := bufio.NewScanner(bytes.NewReader(buf))
for sc.Scan() { for sc.Scan() {
line := sc.Text() line := strings.TrimSpace(sc.Text())
if idx := strings.Index(line, "\trefs/heads/"); idx >= 0 { if idx := strings.Index(line, "\trefs/heads/"); idx >= 0 {
name := line[idx+len("\trefs/heads/"):] name := line[idx+len("\trefs/heads/"):]
if name != "" { if name != "" {
+1 -1
View File
@@ -8,7 +8,7 @@ import (
func TestParseRemoteBranches(t *testing.T) { func TestParseRemoteBranches(t *testing.T) {
t.Parallel() t.Parallel()
input := []byte("aabbccdd\trefs/heads/main\n" + input := []byte("aabbccdd\trefs/heads/main\r\n" +
"eeff0011\trefs/heads/develop\n" + "eeff0011\trefs/heads/develop\n" +
"22334455\trefs/heads/feature/foo\n" + "22334455\trefs/heads/feature/foo\n" +
"deadbeef\trefs/tags/v1.0\n") "deadbeef\trefs/tags/v1.0\n")
+6
View File
@@ -338,6 +338,12 @@ func (s *workspaceService) ListBranches(ctx context.Context, c Caller, wsID uuid
} else if !canRead { } else if !canRead {
return nil, errs.New(errs.CodeForbidden, "no read access") return nil, errs.New(errs.CodeForbidden, "no read access")
} }
if ws.MainPath == "" {
return nil, errs.New(errs.CodeInvalidInput, "workspace not yet cloned")
}
if _, statErr := os.Stat(ws.MainPath); errors.Is(statErr, os.ErrNotExist) {
return nil, errs.New(errs.CodeInvalidInput, "workspace not yet cloned")
}
cred, err := s.loadDecryptedCredential(ctx, wsID) cred, err := s.loadDecryptedCredential(ctx, wsID)
if err != nil { if err != nil {
return nil, err return nil, err
+64 -4
View File
@@ -250,6 +250,22 @@ func (t *trackGit) Checkout(_ context.Context, _, branch string) error {
return nil return nil
} }
type errFetchGit struct {
git.Runner
err error
}
func (e *errFetchGit) FetchRemoteBranch(_ context.Context, _, _ string, _ *git.Credential) error {
return e.err
}
type errCheckoutGit struct {
git.Runner
err error
}
func (e *errCheckoutGit) Checkout(_ context.Context, _, _ string) error { return e.err }
type noopAudit struct{} type noopAudit struct{}
func (noopAudit) Record(_ context.Context, _ audit.Entry) error { return nil } func (noopAudit) Record(_ context.Context, _ audit.Entry) error { return nil }
@@ -404,11 +420,9 @@ func TestCloneRunner_Failure(t *testing.T) {
} }
func TestWorkspaceService_Update_DefaultBranch_Switch(t *testing.T) { func TestWorkspaceService_Update_DefaultBranch_Switch(t *testing.T) {
// 切换 default_branch 时,service 必须先 fetch+checkout 远端分支,再落库。
// 注意:fakeGit 默认 stub 不会记录调用,本测试仅断言最终 DB 状态。
t.Parallel() t.Parallel()
pa := &fakePA{pid: uuid.New(), canRead: true, canWrite: true} pa := &fakePA{pid: uuid.New(), canRead: true, canWrite: true}
fg := &fakeGit{} fg := &trackGit{Runner: &fakeGit{}}
svc, repo := newSvc(t, pa, nil, fg) svc, repo := newSvc(t, pa, nil, fg)
caller := Caller{UserID: uuid.New()} caller := Caller{UserID: uuid.New()}
@@ -426,6 +440,52 @@ func TestWorkspaceService_Update_DefaultBranch_Switch(t *testing.T) {
}) })
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, "develop", repo.wss[wsID].DefaultBranch) require.Equal(t, "develop", repo.wss[wsID].DefaultBranch)
require.Equal(t, []string{"develop"}, fg.fetchRemoteBranchCalls)
require.Equal(t, []string{"develop"}, fg.checkoutCalls)
}
func TestWorkspaceService_Update_DefaultBranch_FetchFails(t *testing.T) {
t.Parallel()
pa := &fakePA{pid: uuid.New(), canRead: true, canWrite: true}
fg := &errFetchGit{Runner: &fakeGit{}, err: errors.New("remote branch not found")}
svc, repo := newSvc(t, pa, nil, fg)
caller := Caller{UserID: uuid.New()}
wsID := uuid.New()
repo.wss[wsID] = &Workspace{
ID: wsID, ProjectID: pa.pid, Slug: "ws1", Name: "W1",
DefaultBranch: "main", MainPath: "/data/ws1/main",
}
newBr := "nonexistent"
_, err := svc.Update(context.Background(), caller, wsID, UpdateWorkspaceInput{
DefaultBranch: &newBr,
})
require.Error(t, err)
// DB should NOT be updated
require.Equal(t, "main", repo.wss[wsID].DefaultBranch)
}
func TestWorkspaceService_Update_DefaultBranch_CheckoutFails(t *testing.T) {
t.Parallel()
pa := &fakePA{pid: uuid.New(), canRead: true, canWrite: true}
fg := &errCheckoutGit{Runner: &fakeGit{}, err: errors.New("dirty working tree")}
svc, repo := newSvc(t, pa, nil, fg)
caller := Caller{UserID: uuid.New()}
wsID := uuid.New()
repo.wss[wsID] = &Workspace{
ID: wsID, ProjectID: pa.pid, Slug: "ws1", Name: "W1",
DefaultBranch: "main", MainPath: "/data/ws1/main",
}
newBr := "develop"
_, err := svc.Update(context.Background(), caller, wsID, UpdateWorkspaceInput{
DefaultBranch: &newBr,
})
require.Error(t, err)
// DB should NOT be updated
require.Equal(t, "main", repo.wss[wsID].DefaultBranch)
} }
func TestWorkspaceService_Update_DefaultBranch_SameValue_NoGitOps(t *testing.T) { func TestWorkspaceService_Update_DefaultBranch_SameValue_NoGitOps(t *testing.T) {
@@ -460,7 +520,7 @@ func TestWorkspaceService_ListBranches(t *testing.T) {
wsID := uuid.New() wsID := uuid.New()
repo.wss[wsID] = &Workspace{ repo.wss[wsID] = &Workspace{
ID: wsID, ProjectID: pa.pid, Slug: "ws1", ID: wsID, ProjectID: pa.pid, Slug: "ws1",
DefaultBranch: "main", MainPath: "/data/ws1/main", DefaultBranch: "main", MainPath: t.TempDir(),
} }
branches, err := svc.ListBranches(context.Background(), caller, wsID) branches, err := svc.ListBranches(context.Background(), caller, wsID)
+3 -4
View File
@@ -52,10 +52,9 @@ export const workspacesApi = {
release: (wtID: string) => release: (wtID: string) =>
request<Worktree>(`/api/v1/worktrees/${enc(wtID)}/release`, { method: 'POST' }), request<Worktree>(`/api/v1/worktrees/${enc(wtID)}/release`, { method: 'POST' }),
listBranches(wsID: string): Promise<string[]> { listBranches: (wsID: string): Promise<string[]> =>
return request<{ branches: string[] }>(`/api/v1/workspaces/${enc(wsID)}/branches`) request<{ branches: string[] }>(`/api/v1/workspaces/${enc(wsID)}/branches`)
.then(r => r.branches) .then(r => r.branches),
},
// ===== Git ops ===== // ===== Git ops =====
statusOnMain: (wsID: string) => statusOnMain: (wsID: string) =>
@@ -36,6 +36,8 @@
:options="baseBranchOptions" :options="baseBranchOptions"
:loading="branchesLoading" :loading="branchesLoading"
clearable clearable
filterable
tag
placeholder="默认使用 default_branch" placeholder="默认使用 default_branch"
/> />
</NFormItem> </NFormItem>