From 7d6e1ff0210fa406e496fc158e3ae3baf0d70b22 Mon Sep 17 00:00:00 2001 From: silverwind <2021+silverwind@noreply.gitea.com> Date: Thu, 5 Nov 2026 04:22:01 +0000 Subject: [PATCH] test: fix TestRunExecListUsesJobEventAndAllPlans hang on macOS (#1378) `TestRunExecListUsesJobEventAndAllPlans` hangs until the test timeout on macOS. Regression from https://gitea.com/gitea/runner/pulls/1338, which added a fifth `runExecList` call to the test to cover `exec --list --detect-event` without events. That took the captured output from 498 to 560 bytes. `captureStdout` swapped `os.Stdout` for a pipe it only read after the callback returned, and on macOS 27.2 an unread pipe holds 512 bytes, so the write blocked forever. Linux pipes hold 64 KiB, so was unaffected. `printList` and `runExecList` now write to a passed writer, `exec --list` hands them `cmd.OutOrStdout()`, and the tests pass a buffer, so no test swaps `os.Stdout` anymore. --------- Co-authored-by: Lunny Xiao Reviewed-on: https://gitea.com/gitea/runner/pulls/1378 Reviewed-by: Lunny Xiao --- internal/app/cmd/exec.go | 15 ++++----- internal/app/cmd/exec_test.go | 57 +++++++++++------------------------ 2 files changed, 25 insertions(+), 47 deletions(-) diff --git a/internal/app/cmd/exec.go b/internal/app/cmd/exec.go index 0dc41305..7f1f9804 100644 --- a/internal/app/cmd/exec.go +++ b/internal/app/cmd/exec.go @@ -10,6 +10,7 @@ import ( "encoding/json/v2" "errors" "fmt" + "io" "maps" "net" "os" @@ -228,7 +229,7 @@ func (i *executeArgs) resolve(path string) string { return path } -func printList(plan *model.Plan) { +func printList(out io.Writer, plan *model.Plan) { type lineInfoDef struct { jobID string jobName string @@ -288,7 +289,7 @@ func printList(plan *model.Plan) { wfNameMaxWidth += 2 wfFileMaxWidth += 2 - fmt.Printf("%*s%*s%*s%*s%*s%*s\n", + fmt.Fprintf(out, "%*s%*s%*s%*s%*s%*s\n", -stageMaxWidth, header.stage, -jobIDMaxWidth, header.jobID, -jobNameMaxWidth, header.jobName, @@ -297,7 +298,7 @@ func printList(plan *model.Plan) { -eventsMaxWidth, header.events, ) for _, line := range lineInfos { - fmt.Printf("%*s%*s%*s%*s%*s%*s\n", + fmt.Fprintf(out, "%*s%*s%*s%*s%*s%*s\n", -stageMaxWidth, line.stage, -jobIDMaxWidth, line.jobID, -jobNameMaxWidth, line.jobName, @@ -307,11 +308,11 @@ func printList(plan *model.Plan) { ) } if duplicateJobIDs { - fmt.Print("\nDetected multiple jobs with the same job name, use `-W` to specify the path to the specific workflow.\n") + fmt.Fprint(out, "\nDetected multiple jobs with the same job name, use `-W` to specify the path to the specific workflow.\n") } } -func runExecList(planner model.WorkflowPlanner, execArgs *executeArgs) error { +func runExecList(out io.Writer, planner model.WorkflowPlanner, execArgs *executeArgs) error { // plan with filtered jobs - to be used for filtering only var filterPlan *model.Plan @@ -347,7 +348,7 @@ func runExecList(planner model.WorkflowPlanner, execArgs *executeArgs) error { return err } - printList(filterPlan) + printList(out, filterPlan) return nil } @@ -405,7 +406,7 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command } if execArgs.runList { - return runExecList(planner, execArgs) + return runExecList(cmd.OutOrStdout(), planner, execArgs) } // plan with triggered jobs diff --git a/internal/app/cmd/exec_test.go b/internal/app/cmd/exec_test.go index a92849cd..6f78b04c 100644 --- a/internal/app/cmd/exec_test.go +++ b/internal/app/cmd/exec_test.go @@ -5,7 +5,6 @@ package cmd import ( "bytes" - "io" "os" "path/filepath" "strings" @@ -182,19 +181,18 @@ func TestRunExecListUsesJobEventAndAllPlans(t *testing.T) { }, } - out := captureStdout(t, func() { - require.NoError(t, runExecList(planner, &executeArgs{job: "build"})) - require.NoError(t, runExecList(planner, &executeArgs{event: "push"})) - require.NoError(t, runExecList(planner, &executeArgs{autodetectEvent: true})) - require.NoError(t, runExecList(planner, &executeArgs{})) - emptyPlanner, err := model.NewWorkflowPlanner(t.TempDir(), false) - require.NoError(t, err) - require.NoError(t, runExecList(emptyPlanner, &executeArgs{autodetectEvent: true})) - }) + var out bytes.Buffer + require.NoError(t, runExecList(&out, planner, &executeArgs{job: "build"})) + require.NoError(t, runExecList(&out, planner, &executeArgs{event: "push"})) + require.NoError(t, runExecList(&out, planner, &executeArgs{autodetectEvent: true})) + require.NoError(t, runExecList(&out, planner, &executeArgs{})) + emptyPlanner, err := model.NewWorkflowPlanner(t.TempDir(), false) + require.NoError(t, err) + require.NoError(t, runExecList(&out, emptyPlanner, &executeArgs{autodetectEvent: true})) - require.Contains(t, out, "Build") - require.Contains(t, out, "Test") - require.Contains(t, out, "Lint") + require.Contains(t, out.String(), "Build") + require.Contains(t, out.String(), "Test") + require.Contains(t, out.String(), "Lint") require.Equal(t, []string{"job:build", "event:push", "event:push", "all"}, planner.calls) } @@ -208,14 +206,13 @@ func TestPrintListReportsDuplicateJobIDs(t *testing.T) { }, }}} - out := captureStdout(t, func() { - printList(plan) - }) + var out bytes.Buffer + printList(&out, plan) - require.Contains(t, out, "Workflow file") - require.Contains(t, out, "Build A") - require.Contains(t, out, "Build B") - require.Contains(t, out, "Detected multiple jobs with the same job name") + require.Contains(t, out.String(), "Workflow file") + require.Contains(t, out.String(), "Build A") + require.Contains(t, out.String(), "Build B") + require.Contains(t, out.String(), "Detected multiple jobs with the same job name") } func TestLoadExecCmdDefinesExpectedFlags(t *testing.T) { @@ -293,26 +290,6 @@ func rawOnNode(event string) yaml.Node { return *node.Content[0] } -func captureStdout(t *testing.T, fn func()) string { - t.Helper() - - old := os.Stdout - r, w, err := os.Pipe() - require.NoError(t, err) - os.Stdout = w - - fn() - - require.NoError(t, w.Close()) - os.Stdout = old - - var buf bytes.Buffer - _, err = io.Copy(&buf, r) - require.NoError(t, err) - require.NoError(t, r.Close()) - return buf.String() -} - func TestExecuteArgsLoadEnvsResultsOrigin(t *testing.T) { t.Setenv("ACTIONS_RESULTS_URL", "") // the default is only supplied when nothing else does handler, err := artifactcache.StartHandler(artifactcache.Options{Dir: t.TempDir(), OutboundIP: "127.0.0.1"})