From c84488888cadcfc7fb3f334ca603deb03750e845 Mon Sep 17 00:00:00 2001 From: Bjoern Kottner <1780890+BjoernKarma@users.noreply.github.com> Date: Thu, 2 Apr 2026 09:19:06 +0000 Subject: [PATCH 1/7] feat: improve error handling - Propagate git execution failures instead of swallowing them - Make config read/parsing errors fail command execution (except config-not-found) - Enable underscore env vars for dotted Viper keys - Update/add tests to lock behavior and prevent regressions - Run full test suite (+ race detector) --- app/cmd/gitpull.go | 4 ++-- app/cmd/gitpull_test.go | 3 ++- app/cmd/gitstatus.go | 4 ++-- app/cmd/gitstatus_test.go | 5 +++-- app/cmd/root.go | 15 ++++++++++--- app/cmd/root_test.go | 42 ++++++++++++++++++++++++++++++++++- gitrepo/gitrepo.go | 18 ++++++++++----- gitrepo/gitrepo_test.go | 9 ++++++++ gitrepo/gitrepos.go | 8 ++++++- gitrepo/gitrepos_test.go | 46 +++++++++++++++++++-------------------- 10 files changed, 113 insertions(+), 41 deletions(-) diff --git a/app/cmd/gitpull.go b/app/cmd/gitpull.go index 9206776..22cf879 100644 --- a/app/cmd/gitpull.go +++ b/app/cmd/gitpull.go @@ -10,7 +10,7 @@ import ( var pullCmd = &cobra.Command{ Use: "pull", Short: "Execute git pull on multiple git repositories.", - Run: func(cmd *cobra.Command, args []string) { - gitrepo.RunGitCommand(gitrepo.GitPull, config.GetBaseDirs()) + RunE: func(cmd *cobra.Command, args []string) error { + return gitrepo.RunGitCommand(gitrepo.GitPull, config.GetBaseDirs()) }, } diff --git a/app/cmd/gitpull_test.go b/app/cmd/gitpull_test.go index d31f08a..f9e3035 100644 --- a/app/cmd/gitpull_test.go +++ b/app/cmd/gitpull_test.go @@ -10,11 +10,12 @@ import ( func TestPullCommandExecutesGitPullOnLocalRepo(t *testing.T) { var buf bytes.Buffer + originalLogWriter := log.Writer() log.SetOutput(&buf) rootCmd.SetOut(&buf) rootCmd.SetErr(&buf) defer func() { - log.SetOutput(nil) + log.SetOutput(originalLogWriter) }() rootCmd.SetArgs([]string{"pull", "--local", "--debug", "--verbose"}) diff --git a/app/cmd/gitstatus.go b/app/cmd/gitstatus.go index 51574f8..b80f09e 100644 --- a/app/cmd/gitstatus.go +++ b/app/cmd/gitstatus.go @@ -10,7 +10,7 @@ import ( var statusCmd = &cobra.Command{ Use: "status", Short: "Execute git status on multiple git repositories.", - Run: func(cmd *cobra.Command, args []string) { - gitrepo.RunGitCommand(gitrepo.GitStatus, config.GetBaseDirs()) + RunE: func(cmd *cobra.Command, args []string) error { + return gitrepo.RunGitCommand(gitrepo.GitStatus, config.GetBaseDirs()) }, } diff --git a/app/cmd/gitstatus_test.go b/app/cmd/gitstatus_test.go index 27b0dc5..50729b0 100644 --- a/app/cmd/gitstatus_test.go +++ b/app/cmd/gitstatus_test.go @@ -10,14 +10,15 @@ import ( func TestStatusCommandExecutesGitStatusOnLocalRepo(t *testing.T) { var buf bytes.Buffer + originalLogWriter := log.Writer() log.SetOutput(&buf) rootCmd.SetOut(&buf) rootCmd.SetErr(&buf) defer func() { - log.SetOutput(nil) + log.SetOutput(originalLogWriter) }() - rootCmd.SetArgs([]string{"status", "--local", "--debug", "--verbose", "--config=gitctl.yaml"}) + rootCmd.SetArgs([]string{"status", "--local", "--debug", "--verbose"}) err := rootCmd.Execute() expected := "Configuration settings:" diff --git a/app/cmd/root.go b/app/cmd/root.go index fdabc20..5551dda 100644 --- a/app/cmd/root.go +++ b/app/cmd/root.go @@ -1,8 +1,10 @@ package cmd import ( - "github.com/bjoernkarma/gitctl/config" "log" + "strings" + + "github.com/bjoernkarma/gitctl/config" "github.com/pkg/errors" @@ -38,7 +40,9 @@ func Execute() error { } func init() { - cobra.OnInitialize(InitConfig) + rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + return InitConfig() + } rootCmd.PersistentFlags().StringVar(&configFile, "config", "", "config file (default is $HOME/.config/gitctl.yaml)") rootCmd.PersistentFlags().BoolVarP(&quiet, "quiet", "q", false, "suppress output") @@ -64,7 +68,7 @@ func init() { rootCmd.AddCommand(pullCmd) } -func InitConfig() { +func InitConfig() error { if configFile != "" { // Use config file from the flag. viper.SetConfigFile(configFile) @@ -76,6 +80,7 @@ func InitConfig() { } // Enable reading from environment variables + viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) viper.AutomaticEnv() // Read the configuration file @@ -83,7 +88,9 @@ func InitConfig() { var configFileNotFoundError viper.ConfigFileNotFoundError if errors.As(err, &configFileNotFoundError) { log.Println("No configuration file found, using defaults and environment variables") + return nil } + return errors.Wrap(err, "failed to read configuration file") } else { log.Printf("Using configuration file: %s", viper.ConfigFileUsed()) } @@ -92,4 +99,6 @@ func InitConfig() { if config.IsDebug() { log.Printf("Configuration settings: %v", viper.AllSettings()) } + + return nil } diff --git a/app/cmd/root_test.go b/app/cmd/root_test.go index 89c8057..58bf63e 100644 --- a/app/cmd/root_test.go +++ b/app/cmd/root_test.go @@ -3,18 +3,27 @@ package cmd import ( "bytes" "log" + "os" + "path/filepath" "testing" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" + + "github.com/bjoernkarma/gitctl/config" ) func TestRootCommandShowsHelp(t *testing.T) { var buf bytes.Buffer + viper.Reset() + originalLogWriter := log.Writer() log.SetOutput(&buf) rootCmd.SetOut(&buf) rootCmd.SetErr(&buf) defer func() { - log.SetOutput(nil) + log.SetOutput(originalLogWriter) + configFile = "" + viper.Reset() }() rootCmd.SetArgs([]string{"--help"}) @@ -24,3 +33,34 @@ func TestRootCommandShowsHelp(t *testing.T) { assert.Contains(t, buf.String(), expected, "expected %v to be contained in %v", expected, buf.String()) assert.NoError(t, err) } + +func TestCommandReturnsErrorForInvalidConfigFile(t *testing.T) { + viper.Reset() + tmpDir := t.TempDir() + invalidConfig := filepath.Join(tmpDir, "gitctl.yaml") + err := os.WriteFile(invalidConfig, []byte("verbosity: ["), 0600) + assert.NoError(t, err) + + rootCmd.SetArgs([]string{"status", "--config", invalidConfig, "--local", "--dryRun"}) + err = rootCmd.Execute() + + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to read configuration file") + + configFile = "" + viper.Reset() +} + +func TestInitConfigReadsUnderscoreEnvVars(t *testing.T) { + viper.Reset() + configFile = "" + t.Setenv("RUN_MODE_LOCAL", "true") + + err := InitConfig() + + assert.NoError(t, err) + assert.True(t, config.IsLocal()) + + configFile = "" + viper.Reset() +} diff --git a/gitrepo/gitrepo.go b/gitrepo/gitrepo.go index 9561370..637e09c 100644 --- a/gitrepo/gitrepo.go +++ b/gitrepo/gitrepo.go @@ -58,13 +58,18 @@ func FindGitRepos(root string) ([]GitRepo, error) { func (gitRepo *GitRepo) RunGitCommand(command string) ([]byte, error) { var verbose = config.IsVerbose() var dryRun = config.IsDryRun() + repoPath := "" + if gitRepo != nil { + repoPath = gitRepo.path + } + if dryRun { - message := fmt.Sprintf("Dry run enabled. Skipping git %s for repository %s", command, gitRepo.path) + message := fmt.Sprintf("Dry run enabled. Skipping git %s for repository %s", command, repoPath) color.PrintSubtleInfo(message) return nil, nil } - if gitRepo == nil || gitRepo.path == "" { + if gitRepo == nil || repoPath == "" { if verbose { color.PrintInfo("The repository path is empty. Skipping the git command.") } @@ -81,10 +86,13 @@ func (gitRepo *GitRepo) RunGitCommand(command string) ([]byte, error) { gitCmd = exec.Command(gitCommand, statusCommand) } - gitCmd.Dir = gitRepo.path - out, _ := gitCmd.CombinedOutput() + gitCmd.Dir = repoPath + out, err := gitCmd.CombinedOutput() // Format the output with headers and separators and color - formattedOutput := FormatOutput(gitRepo.path, out) + formattedOutput := FormatOutput(repoPath, out) + if err != nil { + return []byte(formattedOutput), fmt.Errorf("git %s failed for %s: %w", command, repoPath, err) + } return []byte(formattedOutput), nil } diff --git a/gitrepo/gitrepo_test.go b/gitrepo/gitrepo_test.go index b38db73..98d22d9 100644 --- a/gitrepo/gitrepo_test.go +++ b/gitrepo/gitrepo_test.go @@ -130,3 +130,12 @@ func TestGitRepoRunGitCommand(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, output) } + +func TestGitRepoRunGitStatusWithInvalidRepoPathReturnsError(t *testing.T) { + gitRepo := GitRepo{path: invalidPath} + + output, err := gitRepo.RunGitCommand(GitStatus) + + assert.Error(t, err) + assert.NotNil(t, output) +} diff --git a/gitrepo/gitrepos.go b/gitrepo/gitrepos.go index bb5d5fa..853683b 100644 --- a/gitrepo/gitrepos.go +++ b/gitrepo/gitrepos.go @@ -1,6 +1,7 @@ package gitrepo import ( + "errors" "fmt" "log" @@ -8,10 +9,11 @@ import ( "github.com/bjoernkarma/gitctl/config" ) -func RunGitCommand(command string, baseDirs []string) { +func RunGitCommand(command string, baseDirs []string) error { allGitRepos, err := findGitReposInBaseDirs(baseDirs) if err != nil { log.Println(err) + return err } isVerbose := config.IsVerbose() @@ -20,10 +22,12 @@ func RunGitCommand(command string, baseDirs []string) { if isVerbose && !isQuiet { fmt.Printf("\n============ GIT OUTPUT (VERBOSE) ============\n") } + var commandErrors []error for _, gitRepo := range allGitRepos { output, err := gitRepo.RunGitCommand(command) if err != nil { log.Println(err) + commandErrors = append(commandErrors, err) } if isVerbose && !isQuiet { fmt.Printf("%s", output) @@ -37,6 +41,8 @@ func RunGitCommand(command string, baseDirs []string) { // Print statistics and git output color.PrintGitStatistics() color.PrintGitRepoStatus() + + return errors.Join(commandErrors...) } func findGitReposInBaseDirs(baseDirs []string) ([]GitRepo, error) { diff --git a/gitrepo/gitrepos_test.go b/gitrepo/gitrepos_test.go index 335adcc..73bb63d 100644 --- a/gitrepo/gitrepos_test.go +++ b/gitrepo/gitrepos_test.go @@ -4,62 +4,60 @@ import ( "path/filepath" "testing" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" + + "github.com/bjoernkarma/gitctl/config" ) func TestRunGitStatus(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + viper.Set(config.GitCtlDryRun, true) - // Mock inputs command := GitStatus testDir, _ := filepath.Abs(testDirPath) baseDirs := []string{testDir} - // Call the function under test - RunGitCommand(command, baseDirs) - - // Since RunGitCommand doesn't return anything, we can't make assertions about its return value. - // We could potentially check for side effects (like changes to global state), but without more information, it's hard to say what to check. + err := RunGitCommand(command, baseDirs) + assert.NoError(t, err) } func TestRunGitDefaultCommand(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + viper.Set(config.GitCtlDryRun, true) - // Mock inputs command := "hello" testDir, _ := filepath.Abs(testDirPath) baseDirs := []string{testDir} - // Call the function under test - RunGitCommand(command, baseDirs) - - // Since RunGitCommand doesn't return anything, we can't make assertions about its return value. - // We could potentially check for side effects (like changes to global state), but without more information, it's hard to say what to check. + err := RunGitCommand(command, baseDirs) + assert.NoError(t, err) } func TestRunGitStatusInvalidBaseDirs(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) - // Mock inputs command := GitStatus baseDirs := []string{invalidPath} - // Call the function under test - RunGitCommand(command, baseDirs) - - // Since RunGitCommand doesn't return anything, we can't make assertions about its return value. - // We could potentially check for side effects (like changes to global state), but without more information, it's hard to say what to check. + err := RunGitCommand(command, baseDirs) + assert.Error(t, err) } func TestRunGitPull(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + viper.Set(config.GitCtlDryRun, true) - // Mock inputs command := GitPull testDir, _ := filepath.Abs(testDirPath) baseDirs := []string{testDir} - // Call the function under test - RunGitCommand(command, baseDirs) - - // Since RunGitCommand doesn't return anything, we can't make assertions about its return value. - // We could potentially check for side effects (like changes to global state), but without more information, it's hard to say what to check. + err := RunGitCommand(command, baseDirs) + assert.NoError(t, err) } func TestFindGitReposInBaseDirs(t *testing.T) { From 299c4f0c03456a65808c790cf553c2f33ecd4c91 Mon Sep 17 00:00:00 2001 From: Bjoern Kottner <1780890+BjoernKarma@users.noreply.github.com> Date: Thu, 2 Apr 2026 12:48:03 +0000 Subject: [PATCH 2/7] config: propagate init/base-dir errors and support prefixed env vars --- app/cmd/gitpull.go | 6 +++++- app/cmd/gitstatus.go | 6 +++++- app/cmd/root.go | 15 ++++++++++++-- app/cmd/root_test.go | 17 +++++++++++++++- config/config.go | 26 +++++++++++++++--------- config/config_test.go | 47 ++++++++++++++++++++++++++++++++++--------- config/env.go | 10 ++++++--- config/env_test.go | 20 +++++++++++++++--- config/fork_test.go | 45 ++++------------------------------------- 9 files changed, 122 insertions(+), 70 deletions(-) diff --git a/app/cmd/gitpull.go b/app/cmd/gitpull.go index 22cf879..553c62a 100644 --- a/app/cmd/gitpull.go +++ b/app/cmd/gitpull.go @@ -11,6 +11,10 @@ var pullCmd = &cobra.Command{ Use: "pull", Short: "Execute git pull on multiple git repositories.", RunE: func(cmd *cobra.Command, args []string) error { - return gitrepo.RunGitCommand(gitrepo.GitPull, config.GetBaseDirs()) + baseDirs, err := config.GetBaseDirs() + if err != nil { + return err + } + return gitrepo.RunGitCommand(gitrepo.GitPull, baseDirs) }, } diff --git a/app/cmd/gitstatus.go b/app/cmd/gitstatus.go index b80f09e..4a7c713 100644 --- a/app/cmd/gitstatus.go +++ b/app/cmd/gitstatus.go @@ -11,6 +11,10 @@ var statusCmd = &cobra.Command{ Use: "status", Short: "Execute git status on multiple git repositories.", RunE: func(cmd *cobra.Command, args []string) error { - return gitrepo.RunGitCommand(gitrepo.GitStatus, config.GetBaseDirs()) + baseDirs, err := config.GetBaseDirs() + if err != nil { + return err + } + return gitrepo.RunGitCommand(gitrepo.GitStatus, baseDirs) }, } diff --git a/app/cmd/root.go b/app/cmd/root.go index 5551dda..2a68567 100644 --- a/app/cmd/root.go +++ b/app/cmd/root.go @@ -73,13 +73,24 @@ func InitConfig() error { // Use config file from the flag. viper.SetConfigFile(configFile) } else { + workingDir, err := config.GitctlWorkingDir() + if err != nil { + return errors.Wrap(err, "failed to determine working directory") + } + + configDir, err := config.GitctlConfigDir() + if err != nil { + return errors.Wrap(err, "failed to determine config directory") + } + viper.SetConfigName("gitctl") viper.SetConfigType("yaml") - viper.AddConfigPath(config.GitctlWorkingDir()) - viper.AddConfigPath(config.GitctlConfigDir()) + viper.AddConfigPath(workingDir) + viper.AddConfigPath(configDir) } // Enable reading from environment variables + viper.SetEnvPrefix("GITCTL") viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) viper.AutomaticEnv() diff --git a/app/cmd/root_test.go b/app/cmd/root_test.go index 58bf63e..3a1a1ae 100644 --- a/app/cmd/root_test.go +++ b/app/cmd/root_test.go @@ -54,7 +54,7 @@ func TestCommandReturnsErrorForInvalidConfigFile(t *testing.T) { func TestInitConfigReadsUnderscoreEnvVars(t *testing.T) { viper.Reset() configFile = "" - t.Setenv("RUN_MODE_LOCAL", "true") + t.Setenv("GITCTL_RUN_MODE_LOCAL", "true") err := InitConfig() @@ -64,3 +64,18 @@ func TestInitConfigReadsUnderscoreEnvVars(t *testing.T) { configFile = "" viper.Reset() } + +func TestInitConfigReadsPrefixedVerbosityEnvVars(t *testing.T) { + viper.Reset() + configFile = "" + t.Setenv("GITCTL_VERBOSITY_VERBOSE", "true") + + err := InitConfig() + + assert.NoError(t, err) + assert.True(t, config.IsVerbose()) + + configFile = "" + viper.Reset() +} + diff --git a/config/config.go b/config/config.go index 35f44ea..c83e79a 100644 --- a/config/config.go +++ b/config/config.go @@ -1,27 +1,32 @@ package config import ( + "fmt" "log" "os" "path/filepath" ) // GitctlWorkingDir returns the current gitctl working directory path -func GitctlWorkingDir() string { +func GitctlWorkingDir() (string, error) { workingDir, err := os.Getwd() if err != nil { - log.Fatalf("Error trying to find working directory due to %s", err) + return "", fmt.Errorf("error trying to find working directory: %w", err) } - return workingDir + return workingDir, nil } // GitctlConfigDir returns the gitctl config directory path -func GitctlConfigDir() string { - return filepath.Join(HomeDir(), ".config", "gitctl") +func GitctlConfigDir() (string, error) { + homeDir, err := HomeDir() + if err != nil { + return "", err + } + return filepath.Join(homeDir, ".config", "gitctl"), nil } // HomeDir finds the users home directory -func HomeDir() string { +func HomeDir() (string, error) { // Find home directory. var home string home = os.Getenv("HOME") @@ -35,10 +40,13 @@ func HomeDir() string { } } else { info, err := os.Stat(home) - if err != nil || !info.IsDir() { - log.Fatalf("The path %s is not a valid directory", home) + if err != nil { + return "", fmt.Errorf("failed to stat home directory %s: %w", home, err) + } + if !info.IsDir() { + return "", fmt.Errorf("the path %s is not a valid directory", home) } } - return home + return home, nil } diff --git a/config/config_test.go b/config/config_test.go index 10d5cd5..92c7610 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -8,40 +8,66 @@ import ( func TestHomeDirReturnsHomeEnv(t *testing.T) { homeDir := os.TempDir() - _ = os.Setenv("HOME", homeDir) - result := HomeDir() + t.Setenv("HOME", homeDir) + result, err := HomeDir() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if result != homeDir { t.Errorf("expected %v, got %v", homeDir, result) } } func TestHomeDirHandlesEmptyHomeEnv(t *testing.T) { - _ = os.Setenv("HOME", "") + t.Setenv("HOME", "") homeDir, _ := os.UserHomeDir() tmpDir := os.TempDir() // Alternative in cases where user home dir is not available (e.g. CICD) - result := HomeDir() + result, err := HomeDir() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if result != homeDir && result != tmpDir { t.Errorf("expected %v, got %v", homeDir, result) } } +func TestHomeDirReturnsErrorForInvalidHomeEnv(t *testing.T) { + notADir := filepath.Join(t.TempDir(), "not-a-dir") + err := os.WriteFile(notADir, []byte("x"), 0600) + if err != nil { + t.Fatalf("failed to create file: %v", err) + } + t.Setenv("HOME", notADir) + + _, homeErr := HomeDir() + if homeErr == nil { + t.Fatal("expected error when HOME is not a directory") + } +} + func TestGitctlConfigDirReturnsHomeEnv(t *testing.T) { homeDir := os.TempDir() - _ = os.Setenv("HOME", homeDir) + t.Setenv("HOME", homeDir) configDir := filepath.Join(homeDir, ".config", "gitctl") - result := GitctlConfigDir() + result, err := GitctlConfigDir() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if result != configDir { t.Errorf("expected %v, got %v", homeDir, result) } } func DisableTestGitctlConfigDirHandlesEmptyHomeEnv(t *testing.T) { - _ = os.Setenv("HOME", "") + t.Setenv("HOME", "") homeDir, _ := os.UserHomeDir() configDir := filepath.Join(homeDir, ".config", "gitctl") tmpDir := os.TempDir() // Alternative in cases where user home dir is not available (e.g. CICD) configTempDir := filepath.Join(tmpDir, ".config", "gitctl") - result := GitctlConfigDir() + result, err := GitctlConfigDir() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if result != configDir && result != configTempDir { t.Errorf("expected %v, got %v", homeDir, result) } @@ -49,7 +75,10 @@ func DisableTestGitctlConfigDirHandlesEmptyHomeEnv(t *testing.T) { func TestGitctlWorkingDirReturns(t *testing.T) { workingDir, _ := os.Getwd() - result := GitctlWorkingDir() + result, err := GitctlWorkingDir() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if result != workingDir { t.Errorf("expected %v, got %v", workingDir, result) } diff --git a/config/env.go b/config/env.go index d9522ee..f7c3819 100644 --- a/config/env.go +++ b/config/env.go @@ -62,10 +62,14 @@ func GetConcurrency() string { } // GetBaseDirs returns the base directories as a slice of strings -func GetBaseDirs() []string { +func GetBaseDirs() ([]string, error) { var baseDirs []string if IsLocal() { - baseDirs = []string{GitctlWorkingDir()} + workingDir, err := GitctlWorkingDir() + if err != nil { + return nil, err + } + baseDirs = []string{workingDir} } else { baseDirs = viper.GetStringSlice(GitCtlBaseDirs) } @@ -81,5 +85,5 @@ func GetBaseDirs() []string { } validPaths = append(validPaths, absPath) } - return validPaths + return validPaths, nil } diff --git a/config/env_test.go b/config/env_test.go index 35391d5..3f94bf1 100644 --- a/config/env_test.go +++ b/config/env_test.go @@ -101,20 +101,34 @@ func TestGetConcurrencyReturnsCorrectValue(t *testing.T) { } func TestGetBaseDirsReturnsCorrectValueWhenLocal(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) viper.Set(GitCtlLocal, true) - expected := []string{GitctlWorkingDir()} - result := GetBaseDirs() + workingDir, err := GitctlWorkingDir() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + expected := []string{workingDir} + result, getErr := GetBaseDirs() + if getErr != nil { + t.Fatalf("unexpected error: %v", getErr) + } if len(result) != len(expected) || result[0] != expected[0] { t.Errorf("expected %v, got %v", expected, result) } } func TestGetBaseDirsReturnsCorrectValueWhenNotLocal(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) viper.Set(GitCtlLocal, false) workingDir, _ := os.Getwd() expected := []string{workingDir} viper.Set(GitCtlBaseDirs, expected) - result := GetBaseDirs() + result, err := GetBaseDirs() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if len(result) != len(expected) || result[0] != expected[0] { t.Errorf("expected %v, got %v", expected, result) } diff --git a/config/fork_test.go b/config/fork_test.go index 3efd9c7..df41d3f 100644 --- a/config/fork_test.go +++ b/config/fork_test.go @@ -1,53 +1,16 @@ package config import ( - "bytes" - "fmt" - "os" - "os/exec" "testing" - "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) -// Run a fork test that may crash using os.exit. -func RunForkTest(testName string) (string, string, error) { - // G204: Subprocess launched with a potential tainted input or cmd arguments (gosec) - //nolint:gosec - cmd := exec.Command(os.Args[0], fmt.Sprintf("-test.run=%v", testName)) - cmd.Env = append(os.Environ(), "FORK=1") - - var stdoutB, stderrB bytes.Buffer - cmd.Stdout = &stdoutB - cmd.Stderr = &stderrB - - err := cmd.Run() - - return stdoutB.String(), stderrB.String(), err -} - func TestHomeDirHandlesInvalidHomeDir(t *testing.T) { - if os.Getenv("FORK") == "1" { - _ = os.Setenv("HOME", "/invalid/dir") - HomeDir() - return - } - - stdout, stderr, err := RunForkTest("TestHomeDirHandlesInvalidHomeDir") + t.Setenv("HOME", "/invalid/dir") - assert.NotNil(t, err) - assert.Equal(t, err.Error(), "exit status 1") - assert.Contains(t, stderr, "The path /invalid/dir is not a valid directory") - assert.Contains(t, stdout, "") + _, err := HomeDir() - // Verify ExitCode - var e *exec.ExitError - if errors.As(err, &e) && !e.Success() { - if e.ExitCode() != 1 { - t.Fatalf("process ran with err %v, want exit status 1", err) - } - return - } - t.Fatalf("process ran with err %v, want exit status 1", err) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to stat home directory") } From 73deeafae1be7cd6eef09c17067f8cf8b42ea19f Mon Sep 17 00:00:00 2001 From: Bjoern Kottner <1780890+BjoernKarma@users.noreply.github.com> Date: Thu, 2 Apr 2026 12:48:26 +0000 Subject: [PATCH 3/7] gitrepo: aggregate multi-repo failures and continue scan --- gitrepo/gitrepo_test.go | 6 ++++-- gitrepo/gitrepos.go | 16 ++++++++++------ gitrepo/gitrepos_test.go | 18 ++++++++++++++++-- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/gitrepo/gitrepo_test.go b/gitrepo/gitrepo_test.go index 98d22d9..269ed84 100644 --- a/gitrepo/gitrepo_test.go +++ b/gitrepo/gitrepo_test.go @@ -4,6 +4,7 @@ import ( "log" "os" "path/filepath" + "strings" "testing" "github.com/spf13/viper" @@ -113,9 +114,10 @@ func TestGitRepoRunGitPull(t *testing.T) { output, err := gitRepo.RunGitCommand(GitPull) - // Assert that there was an error and the result is the combined output (standard out/ standard error) - assert.NoError(t, err) + // Pull should propagate git subprocess failures while still returning formatted output. + assert.Error(t, err) assert.NotNil(t, output) + assert.True(t, strings.Contains(err.Error(), "git pull failed")) } func TestGitRepoRunGitCommand(t *testing.T) { diff --git a/gitrepo/gitrepos.go b/gitrepo/gitrepos.go index 853683b..ceb0197 100644 --- a/gitrepo/gitrepos.go +++ b/gitrepo/gitrepos.go @@ -10,10 +10,9 @@ import ( ) func RunGitCommand(command string, baseDirs []string) error { - allGitRepos, err := findGitReposInBaseDirs(baseDirs) - if err != nil { - log.Println(err) - return err + allGitRepos, findErr := findGitReposInBaseDirs(baseDirs) + if findErr != nil { + log.Println(findErr) } isVerbose := config.IsVerbose() @@ -23,6 +22,9 @@ func RunGitCommand(command string, baseDirs []string) error { fmt.Printf("\n============ GIT OUTPUT (VERBOSE) ============\n") } var commandErrors []error + if findErr != nil { + commandErrors = append(commandErrors, findErr) + } for _, gitRepo := range allGitRepos { output, err := gitRepo.RunGitCommand(command) if err != nil { @@ -48,6 +50,7 @@ func RunGitCommand(command string, baseDirs []string) error { func findGitReposInBaseDirs(baseDirs []string) ([]GitRepo, error) { var allGitRepos []GitRepo var verbose = config.IsVerbose() + var findErrors []error for _, baseDir := range baseDirs { if verbose { @@ -57,11 +60,12 @@ func findGitReposInBaseDirs(baseDirs []string) ([]GitRepo, error) { repos, err := FindGitRepos(baseDir) if err != nil { log.Println(err) - return nil, err + findErrors = append(findErrors, fmt.Errorf("failed to find repositories in %s: %w", baseDir, err)) + continue } color.PrintSuccess(fmt.Sprintf("Found %d git directories in %s \n", len(repos), baseDir)) allGitRepos = append(allGitRepos, repos...) } - return allGitRepos, nil + return allGitRepos, errors.Join(findErrors...) } diff --git a/gitrepo/gitrepos_test.go b/gitrepo/gitrepos_test.go index 73bb63d..d251473 100644 --- a/gitrepo/gitrepos_test.go +++ b/gitrepo/gitrepos_test.go @@ -2,6 +2,7 @@ package gitrepo import ( "path/filepath" + "strings" "testing" "github.com/spf13/viper" @@ -70,9 +71,8 @@ func TestFindGitReposInBaseDirs(t *testing.T) { // Assert that there was no error and the result is as expected assert.NoError(t, err) - // Without more information, it's hard to say what the expected result is. - // Here's an example where we just check that the result is not nil. assert.NotNil(t, repos) + assert.Len(t, repos, 1) } func TestFindGitReposInvalidBaseDirs(t *testing.T) { @@ -86,3 +86,17 @@ func TestFindGitReposInvalidBaseDirs(t *testing.T) { assert.Error(t, err) assert.Nil(t, repos) } + +func TestRunGitCommandAggregatesErrorsFromInvalidAndValidBaseDirs(t *testing.T) { + viper.Reset() + t.Cleanup(viper.Reset) + viper.Set(config.GitCtlDryRun, true) + + testDir, _ := filepath.Abs(testDirPath) + baseDirs := []string{invalidPath, testDir} + + err := RunGitCommand(GitStatus, baseDirs) + assert.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "failed to find repositories")) +} + From 59fe679145076c63def3d07fc4712c810dd171ce Mon Sep 17 00:00:00 2001 From: Bjoern Kottner <1780890+BjoernKarma@users.noreply.github.com> Date: Thu, 2 Apr 2026 12:49:16 +0000 Subject: [PATCH 4/7] docs: align config paths and env precedence --- README.md | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index c0d327a..7aa7b76 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,14 @@ To install `gitctl`, follow these steps: ## Configuration -Add a `gitctl.yaml`file the `.config\gitctl` folder in your home directory (`~/.config\gitctl\gitctl.yaml`) with the following format: +`gitctl` searches for `gitctl.yaml` in this order: + +1. Current working directory +2. `~/.config/gitctl/` + +You can also pass an explicit file with `--config /path/to/gitctl.yaml`. + +Create `~/.config/gitctl/gitctl.yaml` with the following format: ```yaml # Verbosity settings @@ -59,6 +66,26 @@ base_dirs: - "//dev//gitctl" ``` +### Environment Variables + +Environment variables use the `GITCTL_` prefix. Dots in config keys are mapped to underscores. + +Examples: + +- `verbosity.verbose` -> `GITCTL_VERBOSITY_VERBOSE` +- `run_mode.local` -> `GITCTL_RUN_MODE_LOCAL` +- `run_mode.dry_run` -> `GITCTL_RUN_MODE_DRY_RUN` +- `output.color` -> `GITCTL_OUTPUT_COLOR` + +### Precedence + +Configuration values are resolved in this order (highest to lowest): + +1. CLI flags +2. Environment variables +3. Config file +4. Built-in defaults + ## Usage Here's how you can use `gitctl`: @@ -88,11 +115,17 @@ Available Commands: status Execute git status on multiple git repositories. Flags: - --config string config file (default is $HOME/gitctl.yaml) + --config string config file (default search: ./gitctl.yaml, then ~/.config/gitctl/gitctl.yaml) -h, --help help for gitctl + -q, --quiet suppress output -v, --verbose verbose output + -d, --debug debug output + -l, --local run with working directory used as base directory + -D, --dryRun run with dry run mode + -c, --color color output (default true) + -C, --concurrency number of concurrent operations (default "1") + --base.dirs base directories for git repositories --version version for gitctl - --viper use Viper for configuration (default true) Use "gitctl [command] --help" for more information about a command. ``` From bff0eb17886869061d993594e0a45dd15e4ccfe4 Mon Sep 17 00:00:00 2001 From: Bjoern Kottner <1780890+BjoernKarma@users.noreply.github.com> Date: Thu, 2 Apr 2026 13:18:13 +0000 Subject: [PATCH 5/7] chore: fix lint and formatting issues --- app/cmd/root_test.go | 1 - color/colorPrinter_test.go | 18 +++++++++--------- gitrepo/gitrepos_test.go | 1 - main.go | 3 ++- main_test.go | 2 +- 5 files changed, 12 insertions(+), 13 deletions(-) diff --git a/app/cmd/root_test.go b/app/cmd/root_test.go index 3a1a1ae..c9c24b9 100644 --- a/app/cmd/root_test.go +++ b/app/cmd/root_test.go @@ -78,4 +78,3 @@ func TestInitConfigReadsPrefixedVerbosityEnvVars(t *testing.T) { configFile = "" viper.Reset() } - diff --git a/color/colorPrinter_test.go b/color/colorPrinter_test.go index ddc9df3..3a56b0a 100644 --- a/color/colorPrinter_test.go +++ b/color/colorPrinter_test.go @@ -14,7 +14,7 @@ import ( const message = "test message" -func expectMessageIsPrinted(t *testing.T, buf bytes.Buffer, message string) { +func expectMessageIsPrinted(t *testing.T, buf bytes.Buffer) { if !bytes.Contains(buf.Bytes(), []byte(message)) { t.Errorf("expected test message to be printed, got %v", buf.String()) } @@ -62,7 +62,7 @@ func TestPrintInfo_ColoredOutput(t *testing.T) { viper.Set(config.GitCtlColor, true) PrintInfo(message) - expectMessageIsPrinted(t, buf, message) + expectMessageIsPrinted(t, buf) } func TestPrintInfo_NonColoredOutput(t *testing.T) { @@ -76,7 +76,7 @@ func TestPrintInfo_NonColoredOutput(t *testing.T) { viper.Set(config.GitCtlColor, false) PrintInfo(message) - expectMessageIsPrinted(t, buf, message) + expectMessageIsPrinted(t, buf) } func TestPrintSubtleInfo_QuietMode(t *testing.T) { @@ -103,7 +103,7 @@ func TestPrintSubtleInfo_ColoredOutput(t *testing.T) { viper.Set(config.GitCtlColor, true) PrintSubtleInfo(message) - expectMessageIsPrinted(t, buf, message) + expectMessageIsPrinted(t, buf) } func TestPrintSubtleInfo_NonColoredOutput(t *testing.T) { @@ -117,7 +117,7 @@ func TestPrintSubtleInfo_NonColoredOutput(t *testing.T) { viper.Set(config.GitCtlColor, false) PrintSubtleInfo(message) - expectMessageIsPrinted(t, buf, message) + expectMessageIsPrinted(t, buf) } func TestPrintSuccess_ColoredOutput(t *testing.T) { @@ -131,7 +131,7 @@ func TestPrintSuccess_ColoredOutput(t *testing.T) { viper.Set(config.GitCtlColor, true) PrintSuccess(message) - expectMessageIsPrinted(t, buf, message) + expectMessageIsPrinted(t, buf) } func TestPrintSuccess_NonColoredOutput(t *testing.T) { @@ -145,7 +145,7 @@ func TestPrintSuccess_NonColoredOutput(t *testing.T) { viper.Set(config.GitCtlColor, false) PrintSuccess(message) - expectMessageIsPrinted(t, buf, message) + expectMessageIsPrinted(t, buf) } func TestPrintError_ColoredOutput(t *testing.T) { @@ -159,7 +159,7 @@ func TestPrintError_ColoredOutput(t *testing.T) { viper.Set(config.GitCtlColor, true) PrintError(message) - expectMessageIsPrinted(t, buf, message) + expectMessageIsPrinted(t, buf) } func TestPrintError_NonColoredOutput(t *testing.T) { @@ -173,7 +173,7 @@ func TestPrintError_NonColoredOutput(t *testing.T) { viper.Set(config.GitCtlColor, false) PrintError(message) - expectMessageIsPrinted(t, buf, message) + expectMessageIsPrinted(t, buf) } // Simulating an error when writing to the output diff --git a/gitrepo/gitrepos_test.go b/gitrepo/gitrepos_test.go index d251473..a06d058 100644 --- a/gitrepo/gitrepos_test.go +++ b/gitrepo/gitrepos_test.go @@ -99,4 +99,3 @@ func TestRunGitCommandAggregatesErrorsFromInvalidAndValidBaseDirs(t *testing.T) assert.Error(t, err) assert.True(t, strings.Contains(err.Error(), "failed to find repositories")) } - diff --git a/main.go b/main.go index 2b5d579..2013f78 100644 --- a/main.go +++ b/main.go @@ -1,8 +1,9 @@ package main import ( - "github.com/bjoernkarma/gitctl/app/cmd" "log" + + "github.com/bjoernkarma/gitctl/app/cmd" ) func main() { diff --git a/main_test.go b/main_test.go index 5e014c1..510a78d 100644 --- a/main_test.go +++ b/main_test.go @@ -5,7 +5,7 @@ import ( "testing" ) -func TestRun(t *testing.T) { +func TestRun(_ *testing.T) { os.Args = []string{"gitctl", "status", "--config", "gitctl.yaml"} main() } From ecf1ccdf6af89a81c11ea6ef16811db4e972a894 Mon Sep 17 00:00:00 2001 From: Bjoern Kottner <1780890+BjoernKarma@users.noreply.github.com> Date: Thu, 2 Apr 2026 13:28:44 +0000 Subject: [PATCH 6/7] test(cmd): make debug-output assertions deterministic in CI --- app/cmd/gitpull_test.go | 1 + app/cmd/gitstatus_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/app/cmd/gitpull_test.go b/app/cmd/gitpull_test.go index f9e3035..e9d6345 100644 --- a/app/cmd/gitpull_test.go +++ b/app/cmd/gitpull_test.go @@ -10,6 +10,7 @@ import ( func TestPullCommandExecutesGitPullOnLocalRepo(t *testing.T) { var buf bytes.Buffer + t.Setenv("GITCTL_VERBOSITY_DEBUG", "true") originalLogWriter := log.Writer() log.SetOutput(&buf) rootCmd.SetOut(&buf) diff --git a/app/cmd/gitstatus_test.go b/app/cmd/gitstatus_test.go index 50729b0..5ee13ba 100644 --- a/app/cmd/gitstatus_test.go +++ b/app/cmd/gitstatus_test.go @@ -10,6 +10,7 @@ import ( func TestStatusCommandExecutesGitStatusOnLocalRepo(t *testing.T) { var buf bytes.Buffer + t.Setenv("GITCTL_VERBOSITY_DEBUG", "true") originalLogWriter := log.Writer() log.SetOutput(&buf) rootCmd.SetOut(&buf) From 8f071df7eda565f492689677cc42d54b677a6ad5 Mon Sep 17 00:00:00 2001 From: Bjoern Kottner <1780890+BjoernKarma@users.noreply.github.com> Date: Thu, 2 Apr 2026 13:36:44 +0000 Subject: [PATCH 7/7] fix(config): always print debug settings even when no config file found --- app/cmd/root.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/cmd/root.go b/app/cmd/root.go index 2a68567..548e465 100644 --- a/app/cmd/root.go +++ b/app/cmd/root.go @@ -99,9 +99,9 @@ func InitConfig() error { var configFileNotFoundError viper.ConfigFileNotFoundError if errors.As(err, &configFileNotFoundError) { log.Println("No configuration file found, using defaults and environment variables") - return nil + } else { + return errors.Wrap(err, "failed to read configuration file") } - return errors.Wrap(err, "failed to read configuration file") } else { log.Printf("Using configuration file: %s", viper.ConfigFileUsed()) }