Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 9 additions & 41 deletions pkg/workflow/github_cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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...)

Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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),
Expand Down
58 changes: 27 additions & 31 deletions pkg/workflow/github_cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
Expand Down