From 3e34a643df13374e38681b864d65f311b7277179 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 25 Mar 2025 16:15:53 +0100 Subject: [PATCH] DEV-877: Split ignorer logic for Smart Build into 2 different functions according to the flag, and use a temporary file for the SHA calculation (#4689) (#4690) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * DEV-877: Made changes to not change the default behavior when the flag is disabled Signed-off-by: Nacho Fuertes * DEV-877: Fix format of the file Signed-off-by: Nacho Fuertes * DEV-877: Speed up SHA calculation creating a temporary file Signed-off-by: Nacho Fuertes * DEV-877: Changed mock to see if a windows unit test is fixed Signed-off-by: Nacho Fuertes * DEV-877: Fix indentation of test file Signed-off-by: Nacho Fuertes * Update pkg/repository/local_git.go Co-authored-by: Javier López Barba --------- Signed-off-by: Nacho Fuertes Co-authored-by: Javier López Barba (cherry picked from commit 2c924ed4c9431d48aa06434783babb104783d2de) Co-authored-by: Ignacio Fuertes --- pkg/repository/git.go | 70 +++++---- pkg/repository/local_git.go | 119 ++++++++++++-- pkg/repository/local_git_test.go | 257 +++++++++++++++++++++++++++---- 3 files changed, 382 insertions(+), 64 deletions(-) diff --git a/pkg/repository/git.go b/pkg/repository/git.go index 3ded9fe92591..fe161afa26d7 100644 --- a/pkg/repository/git.go +++ b/pkg/repository/git.go @@ -35,7 +35,8 @@ import ( ) const ( - gitOperationTimeout = 5 * time.Second + gitOperationTimeout = 5 * time.Second + limitDiffSizeToLogInMB = 1024 * 1024 ) var ( @@ -45,26 +46,28 @@ var ( ) type gitRepoController struct { - repoGetter repositoryGetterInterface - fs afero.Fs - ignorer func(subpath string) ignore.Ignorer - path string + repoGetter repositoryGetterInterface + fs afero.Fs + ignorer func(subpath string) ignore.Ignorer + path string + shouldIgnoreFiles bool } func newGitRepoController(path string) gitRepoController { + shouldIgnoreFiles := env.LoadBoolean("OKTETO_SMART_BUILDS_IGNORE_FILES_ENABLED") return gitRepoController{ repoGetter: gitRepositoryGetter{}, path: path, fs: afero.NewOsFs(), ignorer: func(subpath string) ignore.Ignorer { - if !env.LoadBoolean("OKTETO_SMART_BUILDS_IGNORE_FILES_ENABLED") { + if !shouldIgnoreFiles { return ignore.Never } return ignore.NewMultiIgnorer( ignore.NewDockerIgnorer(filepath.Join(subpath, ".dockerignore")), ) - }, + shouldIgnoreFiles: shouldIgnoreFiles, } } @@ -95,7 +98,7 @@ func (r gitRepoController) calculateIsClean(ctx context.Context) (bool, error) { if err != nil { return false, fmt.Errorf("failed to infer the git repo's current worktree: %w", err) } - status, err := worktree.Status(ctx, "", NewLocalGit("git", &LocalExec{}, r.ignorer)) + status, err := worktree.Status(ctx, "", NewLocalGit("git", &LocalExec{}, r.ignorer, r.shouldIgnoreFiles)) if err != nil { return false, fmt.Errorf("failed to infer the git repo's status: %w", err) } @@ -272,10 +275,6 @@ func (r gitRepoController) GetDiffHash(contextDir string) (string, error) { if err != nil { return "", fmt.Errorf("failed to analyze git repo: %w", err) } - worktree, err := repo.Worktree() - if err != nil { - return "", fmt.Errorf("failed to get git repo root: %w", err) - } // go func that cancels the context after the timeout go func() { @@ -294,7 +293,7 @@ func (r gitRepoController) GetDiffHash(contextDir string) (string, error) { // go func that calculates the diff using git diff go func() { - diff, err := repo.GetDiff(ctx, contextDir, NewLocalGit("git", &LocalExec{}, r.ignorer)) + diff, err := repo.GetDiff(ctx, contextDir, NewLocalGit("git", &LocalExec{}, r.ignorer, r.shouldIgnoreFiles)) select { case <-timeoutCh: oktetoLog.Debug("Timeout exceeded calculating git diff: assuming dirty commit") @@ -320,22 +319,31 @@ func (r gitRepoController) GetDiffHash(contextDir string) (string, error) { return } - filteredUntrackedFiles := []string{} - for _, f := range untrackedFiles { - relpath := resolveRelativePath(filepath.Join(worktree.GetRoot(), f), contextDir) - shouldIgnore, err := r.ignorer(contextDir).Ignore(relpath) + if r.shouldIgnoreFiles { + worktree, err := repo.Worktree() if err != nil { - oktetoLog.Debugf("ignore error in GetDiffHash: %v", err) - } - if !shouldIgnore { - filteredUntrackedFiles = append(filteredUntrackedFiles, f) + oktetoLog.Debugf("Failed to get worktree, skipping calculation of files to ignore: %v", err) } else { - oktetoLog.Debugf("skipping %v file change in GetDiffHash based on known ignore files", f) + filteredUntrackedFiles := []string{} + for _, f := range untrackedFiles { + relpath := resolveRelativePath(filepath.Join(worktree.GetRoot(), f), contextDir) + shouldIgnore, err := r.ignorer(contextDir).Ignore(relpath) + if err != nil { + oktetoLog.Debugf("ignore error in GetDiffHash: %v", err) + } + if !shouldIgnore { + filteredUntrackedFiles = append(filteredUntrackedFiles, f) + } else { + oktetoLog.Debugf("skipping %v file change in GetDiffHash based on known ignore files", f) + } + + } + + untrackedFiles = filteredUntrackedFiles } - } - untrackedFilesContent, err := r.getUntrackedContent(ctx, filteredUntrackedFiles) + untrackedFilesContent, err := r.getUntrackedContent(ctx, untrackedFiles) select { case <-timeoutCh: oktetoLog.Debug("Timeout exceeded calculating git untracked files: assuming dirty commit") @@ -357,7 +365,13 @@ func (r gitRepoController) GetDiffHash(contextDir string) (string, error) { } hashFrom := fmt.Sprintf("%s-%s", diffResponse.diff, untrackedFilesResponse.untrackedFilesDiff) - oktetoLog.Infof("hashing diff: %s", hashFrom) + logDiff := hashFrom + if len(hashFrom) > limitDiffSizeToLogInMB { + logDiff = hashFrom[:limitDiffSizeToLogInMB] + } + + // As the diff might be huge, setting an arbitrary limit in the chars we print in the logs, if not, it could cause performance issues + oktetoLog.Infof("hashing diff: %v", logDiff) diffHash := sha256.Sum256([]byte(hashFrom)) return hex.EncodeToString(diffHash[:]), nil } @@ -386,7 +400,7 @@ func (r gitRepoController) calculateLatestDirSHA(ctx context.Context, contextDir return "", fmt.Errorf("failed to calculate latestDirCommit: failed to analyze git repo: %w", err) } - return repo.GetLatestSHA(ctx, contextDir, NewLocalGit("git", &LocalExec{}, r.ignorer)) + return repo.GetLatestSHA(ctx, contextDir, NewLocalGit("git", &LocalExec{}, r.ignorer, r.shouldIgnoreFiles)) } type repositoryGetterInterface interface { @@ -433,7 +447,7 @@ func (ogr oktetoGitRepository) calculateUntrackedFiles(ctx context.Context, cont return []string{}, fmt.Errorf("failed to infer the git repo's current worktree: %w", err) } - return worktree.ListUntrackedFiles(ctx, contextDir, NewLocalGit("git", &LocalExec{}, nil)) + return worktree.ListUntrackedFiles(ctx, contextDir, NewLocalGit("git", &LocalExec{}, nil, false)) } type oktetoGitWorktree struct { @@ -496,7 +510,7 @@ func (ogr oktetoGitWorktree) ListUntrackedFiles(ctx context.Context, workdir str // git is not available, so we fall back on git-go oktetoLog.Debug("Calculating git untrackedFiles: git is not installed, for better performances consider installing it") - status, err := ogr.Status(ctx, ogr.GetRoot(), NewLocalGit("git", &LocalExec{}, nil)) + status, err := ogr.Status(ctx, ogr.GetRoot(), NewLocalGit("git", &LocalExec{}, nil, false)) if err != nil { return []string{}, fmt.Errorf("failed to infer the git repo's status: %w", err) } diff --git a/pkg/repository/local_git.go b/pkg/repository/local_git.go index 4b57b67bd81c..c3020ba69ea7 100644 --- a/pkg/repository/local_git.go +++ b/pkg/repository/local_git.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "errors" + "fmt" "io" "os" "os/exec" @@ -30,8 +31,10 @@ import ( "github.com/bluekeyes/go-gitdiff/gitdiff" "github.com/go-git/go-git/v5" + "github.com/okteto/okteto/pkg/config" "github.com/okteto/okteto/pkg/ignore" oktetoLog "github.com/okteto/okteto/pkg/log" + "github.com/spf13/afero" ) var ( @@ -147,21 +150,25 @@ type LocalGitInterface interface { } type LocalGit struct { - exec CommandExecutor - ignorer func(subpath string) ignore.Ignorer - gitPath string + exec CommandExecutor + ignorer func(subpath string) ignore.Ignorer + gitPath string + shouldIgnoreFiles bool + fs afero.Fs } -func NewLocalGit(gitPath string, exec CommandExecutor, ignorer func(path string) ignore.Ignorer) *LocalGit { +func NewLocalGit(gitPath string, exec CommandExecutor, ignorer func(path string) ignore.Ignorer, shouldIgnoreFiles bool) *LocalGit { if ignorer == nil { ignorer = func(path string) ignore.Ignorer { return ignore.Never } } return &LocalGit{ - gitPath: gitPath, - exec: exec, - ignorer: ignorer, + gitPath: gitPath, + exec: exec, + ignorer: ignorer, + shouldIgnoreFiles: shouldIgnoreFiles, + fs: afero.NewOsFs(), } } @@ -286,6 +293,16 @@ func (lg *LocalGit) GetDirContentSHA(ctx context.Context, gitPath, dirPath strin return "", errLocalGitCannotGetCommitTooManyAttempts } + if lg.shouldIgnoreFiles { + return lg.getDirContentSHAWithIgnore(ctx, gitPath, dirPath, fixAttempt) + } + + return lg.getDirContentSHAWithoutIgnore(ctx, gitPath, dirPath, fixAttempt) +} + +// getDirContentSHAWithIgnore calculates the SHA of the content of the given directory using git ls-files and git hash-object taking into account +// files to be ignored by the ignorer +func (lg *LocalGit) getDirContentSHAWithIgnore(ctx context.Context, gitPath, dirPath string, fixAttempt int) (string, error) { handleError := func(e error) (string, error) { var exitError *exec.ExitError errors.As(e, &exitError) @@ -306,6 +323,7 @@ func (lg *LocalGit) GetDirContentSHA(ctx context.Context, gitPath, dirPath strin filesRaw, err := lg.exec.RunCommand(ctx, dirPath, lg.gitPath, "--no-optional-locks", "ls-files", "-s") if err != nil { + oktetoLog.Debugf("failed to list files in directory %s: %v", dirPath, err) return handleError(err) } @@ -345,24 +363,104 @@ func (lg *LocalGit) GetDirContentSHA(ctx context.Context, gitPath, dirPath strin filteredLS := strings.Join(filteredLines, "\n") - hashObjectCmdArgs := []string{"--no-optional-locks", "hash-object", "--stdin"} - output, err := lg.exec.RunPipeCommands(ctx, gitPath, "echo", []string{filteredLS}, lg.gitPath, hashObjectCmdArgs) + // As the list of files might be too long, we write it to a file and calculate the hash of the file + f, err := lg.fs.Create(filepath.Join(config.GetOktetoHome(), fmt.Sprintf("git-untracked-%s-%d", filepath.Base(dirPath), time.Now().UnixNano()))) + if err != nil { + oktetoLog.Debugf("failed to create untracked file: %v", err) + return "", err + } + + defer func() { + err := lg.fs.Remove(f.Name()) + if err != nil { + oktetoLog.Debugf("failed to remove untracked file: %v", err) + } + }() + + _, err = f.WriteString(filteredLS) + if err != nil { + oktetoLog.Debugf("failed to write untracked file: %v", err) + return "", err + } + + hashObjectCmdArgs := []string{"--no-optional-locks", "hash-object", f.Name()} + output, err := lg.exec.RunCommand(ctx, dirPath, lg.gitPath, hashObjectCmdArgs...) if err != nil { + oktetoLog.Debugf("failed to calculate hash of directory %s: %v", dirPath, err) return handleError(err) } return string(output), nil } +// getDirContentSHAWithoutIgnore calculates the SHA of the content of the given directory using git ls-files and git hash-object +func (lg *LocalGit) getDirContentSHAWithoutIgnore(ctx context.Context, gitPath, dirPath string, fixAttempt int) (string, error) { + lsFilesCmdArgs := []string{"--no-optional-locks", "ls-files", "-s", dirPath} + hashObjectCmdArgs := []string{"--no-optional-locks", "hash-object", "--stdin"} + + output, err := lg.exec.RunPipeCommands(ctx, gitPath, lg.gitPath, lsFilesCmdArgs, lg.gitPath, hashObjectCmdArgs) + if err != nil { + oktetoLog.Debugf("error getting dir content hash %s: %v", dirPath, err) + var exitError *exec.ExitError + errors.As(err, &exitError) + if exitError != nil { + exitErr := string(exitError.Stderr) + if strings.Contains(exitErr, "detected dubious ownership in repository") { + err = lg.FixDubiousOwnershipConfig(gitPath) + if err != nil { + return "", errLocalGitCannotGetStatusCannotRecover + } + fixAttempt++ + return lg.GetDirContentSHA(ctx, gitPath, dirPath, fixAttempt) + } + } + return "", errLocalGitCannotGetStatusCannotRecover + } + return string(output), nil +} + // Diff returns the diff of the repository at the given path func (lg *LocalGit) Diff(ctx context.Context, gitPath, dirPath string, fixAttempt int) (string, error) { if fixAttempt > 1 { return "", errLocalGitCannotGetCommitTooManyAttempts } + if lg.shouldIgnoreFiles { + return lg.diffWithIgnore(ctx, gitPath, dirPath, fixAttempt) + } + + return lg.diffWithoutIgnore(ctx, gitPath, dirPath, fixAttempt) +} + +// diffWithoutIgnore calculates the diff of the repository at the given path +func (lg *LocalGit) diffWithoutIgnore(ctx context.Context, gitPath, dirPath string, fixAttempt int) (string, error) { + output, err := lg.exec.RunCommand(ctx, gitPath, lg.gitPath, "--no-optional-locks", "diff", "--no-color", "--", "HEAD", dirPath) + if err != nil { + oktetoLog.Debugf("error getting diff of directory %s: %v", dirPath, err) + var exitError *exec.ExitError + errors.As(err, &exitError) + if exitError != nil { + exitErr := string(exitError.Stderr) + if strings.Contains(exitErr, "detected dubious ownership in repository") { + err = lg.FixDubiousOwnershipConfig(gitPath) + if err != nil { + return "", errLocalGitCannotGetStatusCannotRecover + } + fixAttempt++ + return lg.GetDirContentSHA(ctx, gitPath, dirPath, fixAttempt) + } + } + return "", errLocalGitCannotGetStatusCannotRecover + } + return string(output), nil +} + +// diffWithIgnore calculates the diff of the repository at the given path taking into account files to be ignored by the ignorer +func (lg *LocalGit) diffWithIgnore(ctx context.Context, gitPath, dirPath string, fixAttempt int) (string, error) { rawOutput, err := lg.exec.RunCommand(ctx, dirPath, lg.gitPath, "--no-optional-locks", "diff", "--no-color", "HEAD", ".") if err != nil { + oktetoLog.Debugf("error getting diff of directory %s: %v", dirPath, err) var exitError *exec.ExitError errors.As(err, &exitError) if exitError != nil { @@ -381,6 +479,7 @@ func (lg *LocalGit) Diff(ctx context.Context, gitPath, dirPath string, fixAttemp files, _, err := gitdiff.Parse(bytes.NewReader(rawOutput)) if err != nil { + oktetoLog.Debugf("error parsing diff of directory %s: %v", dirPath, err) return "", err } @@ -407,7 +506,7 @@ func (lg *LocalGit) Diff(ctx context.Context, gitPath, dirPath string, fixAttemp return diff.String(), nil } -// relpath removes dirPathToRemove from fullpath and returns the relative path +// resolveRelativePath removes dirPathToRemove from absoluteFullpath and returns the relative path func resolveRelativePath(absoluteFullpath string, dirPathToRemove string) string { relpath := strings.TrimPrefix(absoluteFullpath, dirPathToRemove) return strings.TrimPrefix(relpath, string(filepath.Separator)) diff --git a/pkg/repository/local_git_test.go b/pkg/repository/local_git_test.go index 901cb4eb7626..28a50a0a2dad 100644 --- a/pkg/repository/local_git_test.go +++ b/pkg/repository/local_git_test.go @@ -16,13 +16,44 @@ package repository import ( "context" "os/exec" + "path/filepath" "runtime" "testing" "time" + "github.com/okteto/okteto/pkg/ignore" + "github.com/spf13/afero" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" ) +type mockLocalGit struct { + mock.Mock +} + +func (mlg *mockLocalGit) RunCommand(ctx context.Context, dir string, name string, arg ...string) ([]byte, error) { + args := mlg.Called(ctx, dir, name, arg) + return args.Get(0).([]byte), args.Error(1) +} + +func (mlg *mockLocalGit) LookPath(file string) (string, error) { + args := mlg.Called(file) + return args.String(0), args.Error(1) +} + +func (mlg *mockLocalGit) RunPipeCommands(ctx context.Context, dir string, cmd1 string, cmd1Args []string, cmd2 string, cmd2Args []string) ([]byte, error) { + args := mlg.Called(ctx, dir, cmd1, cmd1Args, cmd2, cmd2Args) + + return args.Get(0).([]byte), args.Error(1) +} + +type fakeIgnorer struct{} + +func (fi *fakeIgnorer) Ignore(path string) (bool, error) { + return path == filepath.Join("git", "README.md"), nil +} + type mockLocalExec struct { runCommand func(ctx context.Context, dir string, name string, arg ...string) ([]byte, error) pipeCommand func(ctx context.Context, dir string, cmd1 string, cmd1Args []string, cmd2 string, cmd2Args []string) ([]byte, error) @@ -83,7 +114,7 @@ func TestLocalGit_Exists(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - lg := NewLocalGit("git", tt.mockExec(), nil) + lg := NewLocalGit("git", tt.mockExec(), nil, false) _, err := lg.Exists() assert.ErrorIs(t, err, tt.err) }) @@ -122,7 +153,7 @@ func TestLocalGit_FixDubiousOwnershipConfig(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - lg := NewLocalGit("git", tt.mockExec(), nil) + lg := NewLocalGit("git", tt.mockExec(), nil, false) err := lg.FixDubiousOwnershipConfig("/test/dir") assert.ErrorIs(t, err, tt.err) @@ -210,7 +241,7 @@ func TestLocalGit_Status(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - lg := NewLocalGit("git", tt.mock(), nil) + lg := NewLocalGit("git", tt.mock(), nil, false) _, err := lg.Status(context.Background(), "/test/dir", "", tt.fixAttempts) assert.ErrorIs(t, err, tt.expectedErr) @@ -246,32 +277,13 @@ func Test_LocalExec_RunCommand(t *testing.T) { assert.Equal(t, "okteto\n", string(got)) } -func TestLocalGit_GetDirContentSHA(t *testing.T) { +func TestLocalGit_GetDirContentSHAWithError(t *testing.T) { tests := []struct { expectedErr error mock func() *mockLocalExec name string fixAttempts int }{ - { - name: "success", - fixAttempts: 0, - mock: func() *mockLocalExec { - return &mockLocalExec{ - runCommand: func(ctx context.Context, dir, name string, arg ...string) ([]byte, error) { - return []byte( - `100644 5cc5ccc76f0fa2674fd3b17c1b863d62eebcb853 0 Dockerfile - 100644 261eeb9e9f8b2b4b0d119366dda99c6fd7d35c64 0 LICENSE - 100644 50675ea7dda5ea4d4204468eaf121681c204a717 0 Makefile - 100644 5a48ac3289fbec053cc3016b9f1a46d7d59597d2 0 README.md`), nil - }, - pipeCommand: func(ctx context.Context, dir string, cmd1 string, cmd1Args []string, cmd2 string, cmd2Args []string) ([]byte, error) { - return []byte("123123"), nil - }, - } - }, - expectedErr: nil, - }, { name: "failure due to too many attempts", fixAttempts: 2, @@ -289,7 +301,7 @@ func TestLocalGit_GetDirContentSHA(t *testing.T) { fixAttempts: 1, mock: func() *mockLocalExec { return &mockLocalExec{ - runCommand: func(ctx context.Context, dir string, name string, arg ...string) ([]byte, error) { + pipeCommand: func(ctx context.Context, dir string, cmd1 string, cmd1Args []string, cmd2 string, cmd2Args []string) ([]byte, error) { return nil, assert.AnError }, } @@ -300,13 +312,62 @@ func TestLocalGit_GetDirContentSHA(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - lg := NewLocalGit("git", tt.mock(), nil) + lg := NewLocalGit("git", tt.mock(), nil, false) _, err := lg.GetDirContentSHA(context.Background(), "", "/test/dir", tt.fixAttempts) assert.ErrorIs(t, err, tt.expectedErr) }) } } + +func TestLocalGit_GetDirContentSHAWithoutIgnore(t *testing.T) { + dirPath := "/test/dir" + gitPath := "git" + lsFilesCmdArgs := []string{"--no-optional-locks", "ls-files", "-s", dirPath} + hashObjectCmdArgs := []string{"--no-optional-locks", "hash-object", "--stdin"} + localGit := &mockLocalGit{} + + localGit.On("RunPipeCommands", mock.Anything, gitPath, gitPath, lsFilesCmdArgs, gitPath, hashObjectCmdArgs).Return([]byte("very complex SHA"), nil) + + lg := NewLocalGit(gitPath, localGit, nil, false) + + output, err := lg.GetDirContentSHA(context.Background(), gitPath, dirPath, 0) + + localGit.AssertExpectations(t) + require.NoError(t, err) + require.Equal(t, "very complex SHA", output) + +} + +func TestLocalGit_GetDirContentSHAWithIgnore(t *testing.T) { + dirPath := "/test/dir" + gitPath := "git" + localGit := &mockLocalGit{} + lsFilesOutput := `100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 README.md +100644 f8d0d1b317c27e75dc328e1e33d2ac8ed257db44 0 main.go +100644 a45c22d80f57524e6b605e842a48bde9c455c8f0 0 pkg/utils/helpers.go +100644 2c9be049f5eb50b3c2d03de362e8f4d3e0b96fb4 0 cmd/root.go +100644 8f7e0670619e3f6d83731c99df68307d5273b82c 0 .gitignore +100755 f8d0d1b317c27e75dc328e1e33d2ac8ed257db44 0 scripts/build.sh` + ignorer := func(filename string) ignore.Ignorer { + return &fakeIgnorer{} + } + + localGit.On("RunCommand", mock.Anything, dirPath, gitPath, []string{"--no-optional-locks", "ls-files", "-s"}).Return([]byte(lsFilesOutput), nil) + + localGit.On("RunCommand", mock.Anything, dirPath, gitPath, mock.Anything).Return([]byte("very complex SHA with ignorer"), nil) + + lg := NewLocalGit(gitPath, localGit, ignorer, true) + lg.fs = afero.NewMemMapFs() + + output, err := lg.GetDirContentSHA(context.Background(), gitPath, dirPath, 0) + + localGit.AssertExpectations(t) + require.NoError(t, err) + require.Equal(t, "very complex SHA with ignorer", output) + +} + func TestLocalGit_ListUntrackedFiles(t *testing.T) { tests := []struct { expectedErr error @@ -375,7 +436,7 @@ func TestLocalGit_ListUntrackedFiles(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - lg := NewLocalGit("git", tt.execMock(), nil) + lg := NewLocalGit("git", tt.execMock(), nil, false) files, err := lg.ListUntrackedFiles(context.Background(), "/test/dir", "/test", tt.fixAttempt) assert.Equal(t, tt.expectedFiles, files) @@ -383,3 +444,147 @@ func TestLocalGit_ListUntrackedFiles(t *testing.T) { }) } } + +func TestLocalGit_GetDiffWithError(t *testing.T) { + tests := []struct { + expectedErr error + mock func() *mockLocalExec + name string + fixAttempts int + }{ + { + name: "failure due to too many attempts", + fixAttempts: 2, + mock: func() *mockLocalExec { + return &mockLocalExec{ + runCommand: func(_ context.Context, _ string, _ string, _ ...string) ([]byte, error) { + return nil, assert.AnError + }, + } + }, + expectedErr: errLocalGitCannotGetCommitTooManyAttempts, + }, + { + name: "cannot recover", + fixAttempts: 1, + mock: func() *mockLocalExec { + return &mockLocalExec{ + runCommand: func(_ context.Context, _ string, _ string, _ ...string) ([]byte, error) { + return nil, assert.AnError + }, + } + }, + expectedErr: errLocalGitCannotGetStatusCannotRecover, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + lg := NewLocalGit("git", tt.mock(), nil, false) + _, err := lg.Diff(context.Background(), "", "/test/dir", tt.fixAttempts) + + assert.ErrorIs(t, err, tt.expectedErr) + }) + } +} + +func TestLocalGit_GetDiffWithoutIgnore(t *testing.T) { + dirPath := "/test/dir" + gitPath := "git" + diffCmdArgs := []string{"--no-optional-locks", "diff", "--no-color", "--", "HEAD", dirPath} + localGit := &mockLocalGit{} + + localGit.On("RunCommand", mock.Anything, gitPath, gitPath, diffCmdArgs).Return([]byte("very complex diff"), nil) + + lg := NewLocalGit(gitPath, localGit, nil, false) + + output, err := lg.Diff(context.Background(), gitPath, dirPath, 0) + + localGit.AssertExpectations(t) + require.NoError(t, err) + require.Equal(t, "very complex diff", output) +} + +func TestLocalGit_GetDiffWithIgnore(t *testing.T) { + dirPath := "/test/dir" + gitPath := "git" + diffCmdArgs := []string{"--no-optional-locks", "diff", "--no-color", "HEAD", "."} + diffOutput := `diff --git a/README.md b/README.md +index d14a7f3..3c9e1a0 100644 +--- a/README.md ++++ b/README.md +@@ -1,5 +1,11 @@ +-# My Project +-This project does something useful. +- +-## Getting Started +-To get started, run "main.go". ++# My Awesome Project ++ ++This project does something incredibly useful and efficient. ++ ++## Requirements ++ ++- Go 1.20 or higher ++- Git ++ ++## Getting Started ++Run the application using: "go run main.go" + +diff --git a/main.go b/main.go +index e69de29..b6fc4c3 100644 +--- a/main.go ++++ b/main.go +@@ -0,0 +1,15 @@ ++package main ++ ++import ( ++ "fmt" ++ "os" ++) ++ ++func main() { ++ name := "World" ++ if len(os.Args) > 1 { ++ name = os.Args[1] ++ } ++ fmt.Printf("Hello, %s!\n", name) ++} ++` + expectedOutput := `diff --git a/main.go b/main.go +index e69de29..b6fc4c3 100644 +--- a/main.go ++++ b/main.go +@@ -0,0 +1,15 @@ ++package main ++ ++import ( ++ "fmt" ++ "os" ++) ++ ++func main() { ++ name := "World" ++ if len(os.Args) > 1 { ++ name = os.Args[1] ++ } ++ fmt.Printf("Hello, %s!\n", name) ++} ++ +\ No newline at end of file +` + localGit := &mockLocalGit{} + + localGit.On("RunCommand", mock.Anything, dirPath, gitPath, diffCmdArgs).Return([]byte(diffOutput), nil) + + ignorer := func(filename string) ignore.Ignorer { + return &fakeIgnorer{} + } + lg := NewLocalGit(gitPath, localGit, ignorer, true) + + output, err := lg.Diff(context.Background(), gitPath, dirPath, 0) + + localGit.AssertExpectations(t) + require.NoError(t, err) + require.Equal(t, expectedOutput, output) +}