mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-10-02 09:24:52 +08:00
fix(diff): propagate untracked file listing errors (#1140)
* fix(diff): propagate untracked file listing errors * test: Apply suggestion from @lizhengfeng101 --------- Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
This commit is contained in:
@@ -620,8 +620,11 @@ func (p *Provider) untrackedFileDiffs(ctx context.Context) ([]string, error) {
|
||||
}
|
||||
|
||||
func (p *Provider) untrackedFilesList(ctx context.Context) ([]string, error) {
|
||||
out, err := p.runGit(ctx, "-c", "core.quotepath=false", "ls-files", "--others", "--exclude-standard")
|
||||
if err != nil || out == "" {
|
||||
out, stderr, err := p.runGitSplit(ctx, "-c", "core.quotepath=false", "ls-files", "--others", "--exclude-standard")
|
||||
if err != nil {
|
||||
return nil, gitFailure("git ls-files", stderr, err)
|
||||
}
|
||||
if out == "" {
|
||||
return nil, nil
|
||||
}
|
||||
patterns := p.loadGitignorePatterns()
|
||||
|
||||
@@ -134,6 +134,27 @@ func TestGetDiff_WorkspaceFailureSurfacesFallbackMessage(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestUntrackedFilesListPropagatesGitFailure separates the two cases the old
|
||||
// `if err != nil || out == ""` folded together: a repository with no untracked
|
||||
// files, and git failing to enumerate them at all. The second one used to
|
||||
// return an empty list, so a workspace review carried on with the untracked
|
||||
// half of its input missing and no way for the caller to notice.
|
||||
func TestUntrackedFilesListPropagatesGitFailure(t *testing.T) {
|
||||
repo := filepath.Join(t.TempDir(), "missing-repo")
|
||||
|
||||
provider := NewWorkspaceProvider(repo, nil)
|
||||
|
||||
_, err := provider.untrackedFilesList(context.Background())
|
||||
if err == nil {
|
||||
t.Fatal("expected untrackedFilesList to return git error")
|
||||
}
|
||||
// Pins gitFailure rather than any error: a bare fmt.Errorf would satisfy the
|
||||
// check above while dropping git's own diagnosis, which is the #972 regression.
|
||||
if got := err.Error(); !strings.Contains(got, "git ls-files failed") {
|
||||
t.Errorf("error %q lost the operation name", got)
|
||||
}
|
||||
}
|
||||
|
||||
// shimGit puts a fake `git` at the front of PATH for the duration of the test.
|
||||
func shimGit(t *testing.T, body string) {
|
||||
t.Helper()
|
||||
|
||||
Reference in New Issue
Block a user