diff --git a/pkg/workflow/github_cli.go b/pkg/workflow/github_cli.go index 787d33abfca..91b78caed23 100644 --- a/pkg/workflow/github_cli.go +++ b/pkg/workflow/github_cli.go @@ -28,18 +28,14 @@ func setupGHCommand(ctx context.Context, args ...string) *exec.Cmd { var cmd *exec.Cmd if ctx != nil { cmd = exec.CommandContext(ctx, "gh", args...) - if ghToken != "" || githubToken != "" { - githubCLILog.Printf("Using gh CLI via go-gh/v2 for command with context: gh %v", args) - } else { - githubCLILog.Printf("No token available, using default gh CLI with context for command: gh %v", args) - } } else { cmd = exec.Command("gh", args...) - if ghToken != "" || githubToken != "" { - githubCLILog.Printf("Using gh CLI via go-gh/v2 for command: gh %v", args) - } else { - githubCLILog.Printf("No token available, using default gh CLI for command: gh %v", args) - } + } + + if ghToken != "" || githubToken != "" { + githubCLILog.Printf("Token detected, using gh CLI for command: gh %v", args) + } else { + githubCLILog.Printf("No token available, using default gh CLI for command: gh %v", args) } // Set up environment to ensure token is available @@ -97,7 +93,7 @@ func enrichGHError(err error) error { } // runGHWithSpinnerContext executes a gh CLI command with context support, a spinner, -// and returns the output. This is the core implementation for RunGHContext. +// and returns the output. This is the core implementation for all RunGH* functions. func runGHWithSpinnerContext(ctx context.Context, spinnerMessage string, combined bool, args ...string) ([]byte, error) { cmd := ExecGHContext(ctx, args...) @@ -124,34 +120,6 @@ func runGHWithSpinnerContext(ctx context.Context, spinnerMessage string, combine return output, enrichGHError(err) } -// runGHWithSpinner executes a gh CLI command with a spinner and returns the output. -// This is the core implementation shared by RunGH and RunGHCombined. -func runGHWithSpinner(spinnerMessage string, combined bool, args ...string) ([]byte, error) { - cmd := ExecGH(args...) - - // Show spinner in interactive terminals - if tty.IsStderrTerminal() { - spinner := console.NewSpinner(spinnerMessage) - spinner.Start() - var output []byte - var err error - if combined { - output, err = cmd.CombinedOutput() - } else { - output, err = cmd.Output() - err = enrichGHError(err) - } - spinner.Stop() - return output, err - } - - if combined { - return cmd.CombinedOutput() - } - output, err := cmd.Output() - return output, enrichGHError(err) -} - // RunGH executes a gh CLI command with a spinner and returns the stdout output. // The spinner is shown in interactive terminals to provide feedback during network operations. // The spinnerMessage parameter describes what operation is being performed. @@ -160,7 +128,7 @@ func runGHWithSpinner(spinnerMessage string, combined bool, args ...string) ([]b // // output, err := RunGH("Fetching user info...", "api", "/user") func RunGH(spinnerMessage string, args ...string) ([]byte, error) { - return runGHWithSpinner(spinnerMessage, false, args...) + return RunGHContext(context.Background(), spinnerMessage, args...) } // RunGHContext executes a gh CLI command with context support (for cancellation/timeout), a @@ -182,7 +150,7 @@ func RunGHContext(ctx context.Context, spinnerMessage string, args ...string) ([ // // output, err := RunGHCombined("Creating repository...", "repo", "create", "myrepo") func RunGHCombined(spinnerMessage string, args ...string) ([]byte, error) { - return runGHWithSpinner(spinnerMessage, true, args...) + return RunGHCombinedContext(context.Background(), spinnerMessage, args...) } // RunGHCombinedContext executes a gh CLI command with context support (for cancellation/timeout), diff --git a/pkg/workflow/github_cli_test.go b/pkg/workflow/github_cli_test.go index 89574c3637a..1e3b15b7816 100644 --- a/pkg/workflow/github_cli_test.go +++ b/pkg/workflow/github_cli_test.go @@ -341,10 +341,8 @@ func TestSetupGHCommand(t *testing.T) { } } -// TestRunGHWithSpinner tests the core runGHWithSpinner function -// Note: This test validates the function exists and handles arguments correctly -// Actual spinner behavior is tested via RunGH and RunGHCombined -func TestRunGHWithSpinnerHelperExists(t *testing.T) { +// TestRunGHFunctions tests that RunGH and RunGHCombined delegate correctly to their context variants. +func TestRunGHFunctions(t *testing.T) { // Save original environment originalGHToken := os.Getenv("GH_TOKEN") originalGitHubToken := os.Getenv("GITHUB_TOKEN") @@ -353,37 +351,35 @@ func TestRunGHWithSpinnerHelperExists(t *testing.T) { os.Setenv("GITHUB_TOKEN", originalGitHubToken) }() - // Set up test environment - no tokens so command won't actually execute + // Set up test environment with no tokens to keep behavior deterministic for unit tests. os.Unsetenv("GH_TOKEN") os.Unsetenv("GITHUB_TOKEN") - // Test that the function exists and can be called - // We use a command that will fail quickly without credentials - // to verify the integration works - tests := []struct { - name string - combined bool - }{ - { - name: "Test stdout mode", - combined: false, - }, - { - name: "Test combined mode", - combined: true, - }, - } + t.Run("RunGH matches RunGHContext", func(t *testing.T) { + gotOut, gotErr := RunGH("Test spinner...", "auth", "status") + wantOut, wantErr := RunGHContext(context.Background(), "Test spinner...", "auth", "status") + + assert.Equal(t, wantOut, gotOut, "RunGH should return the same output as RunGHContext") + if wantErr == nil { + assert.NoError(t, gotErr, "RunGH should match RunGHContext error behavior") + } else { + require.Error(t, gotErr, "RunGH should match RunGHContext error behavior") + assert.Equal(t, wantErr.Error(), gotErr.Error(), "RunGH should return the same error text as RunGHContext") + } + }) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Just verify the function can be called - // We expect it to fail since gh command requires auth - _, err := runGHWithSpinner("Test spinner...", tt.combined, "auth", "status") - // We don't care about the error - we just want to verify the function exists - // and doesn't panic when called - _ = err - }) - } + t.Run("RunGHCombined matches RunGHCombinedContext", func(t *testing.T) { + gotOut, gotErr := RunGHCombined("Test spinner...", "auth", "status") + wantOut, wantErr := RunGHCombinedContext(context.Background(), "Test spinner...", "auth", "status") + + assert.Equal(t, wantOut, gotOut, "RunGHCombined should return the same output as RunGHCombinedContext") + if wantErr == nil { + assert.NoError(t, gotErr, "RunGHCombined should match RunGHCombinedContext error behavior") + } else { + require.Error(t, gotErr, "RunGHCombined should match RunGHCombinedContext error behavior") + assert.Equal(t, wantErr.Error(), gotErr.Error(), "RunGHCombined should return the same error text as RunGHCombinedContext") + } + }) } // TestEnrichGHError tests that enrichGHError appends stderr from *exec.ExitError