Skip to content

Commit 0824058

Browse files
znullCopilot
andcommitted
Add tests pinning command stdout fd-pass fast path
One of the optimizations enabled by the Stage interface redesign in #21 is that when a commandStage at the end of a pipeline has its stdout pointed at an *os.File, exec.Cmd dup's that fd straight into the child process. The PR called out two benefits: - the Go-side copy stage between subprocess and Pipeline.stdout becomes unnecessary (wasteful goroutine + buffer); - the subprocess can detect when Pipeline.stdout is closed, which the old intermediate pipe hid from it. Until now nothing asserted this contract directly. The pipe-type negotiation tests in pipe_matching_test.go only check that the right kind of pipe was chosen between mock stages; the existing Command + WithStdoutCloser(*os.File) test would still pass even if a future refactor silently wrapped stdout in a Go-side io.Copy. Pin the contract with two tests (one direct, one through Pipeline) that assert s.cmd.Stdout == userProvidedFile after Start() for both WithStdoutCloser(*os.File) and WithStdout(*os.File) (writerNopCloser- wrapped). A regression would now fail loudly with a message that names the optimization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 2fd4f7c commit 0824058

1 file changed

Lines changed: 118 additions & 0 deletions

File tree

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
package pipe
2+
3+
import (
4+
"context"
5+
"io"
6+
"os"
7+
"os/exec"
8+
"testing"
9+
)
10+
11+
// TestCommandStageStdoutFastPath asserts that when a commandStage's stdout is
12+
// an `*os.File`, the file is set as `cmd.Stdout` so that `exec.Cmd` dup's the
13+
// fd into the child process directly. This is one of the optimizations enabled
14+
// by the Stage interface redesign in #21: the subprocess writes straight to
15+
// the caller's destination fd with no Go-side copy stage in between, and the
16+
// subprocess can detect when that fd is closed.
17+
func TestCommandStageStdoutFastPath(t *testing.T) {
18+
cases := []struct {
19+
name string
20+
wrap func(*os.File) io.WriteCloser
21+
}{
22+
{
23+
name: "raw *os.File via WithStdoutCloser",
24+
wrap: func(f *os.File) io.WriteCloser { return f },
25+
},
26+
{
27+
name: "writerNopCloser{*os.File} via WithStdout",
28+
wrap: func(f *os.File) io.WriteCloser { return writerNopCloser{f} },
29+
},
30+
}
31+
for _, tc := range cases {
32+
t.Run(tc.name, func(t *testing.T) {
33+
t.Parallel()
34+
ctx, cancel := context.WithCancel(context.Background())
35+
defer cancel()
36+
37+
f, err := os.CreateTemp(t.TempDir(), "stdout")
38+
if err != nil {
39+
t.Fatal(err)
40+
}
41+
t.Cleanup(func() { _ = f.Close() })
42+
43+
cmd := exec.Command("true")
44+
s := CommandStage("true", cmd).(*commandStage)
45+
46+
if err := s.Start(ctx, Env{}, nil, tc.wrap(f)); err != nil {
47+
t.Fatalf("Start: %v", err)
48+
}
49+
t.Cleanup(func() { _ = s.Wait() })
50+
51+
gotFile, ok := s.cmd.Stdout.(*os.File)
52+
if !ok {
53+
t.Fatalf("expected cmd.Stdout to be *os.File, got %T", s.cmd.Stdout)
54+
}
55+
if gotFile != f {
56+
t.Errorf("expected cmd.Stdout to be the user-provided *os.File "+
57+
"(fd %d), got a different *os.File (fd %d). The fd-pass "+
58+
"fast path is broken; sendfile/zero-copy will not apply.",
59+
f.Fd(), gotFile.Fd())
60+
}
61+
})
62+
}
63+
}
64+
65+
// TestCommandStageStdoutFastPathThroughPipeline is the same assertion
66+
// but driven end-to-end through `Pipeline.Start()`, so it also
67+
// exercises the `Pipeline.stdout` plumbing that hands the writer to
68+
// the last stage.
69+
func TestCommandStageStdoutFastPathThroughPipeline(t *testing.T) {
70+
cases := []struct {
71+
name string
72+
option func(*os.File) Option
73+
}{
74+
{
75+
name: "WithStdoutCloser(*os.File)",
76+
option: func(f *os.File) Option { return WithStdoutCloser(f) },
77+
},
78+
{
79+
name: "WithStdout(*os.File)",
80+
option: func(f *os.File) Option { return WithStdout(f) },
81+
},
82+
}
83+
for _, tc := range cases {
84+
t.Run(tc.name, func(t *testing.T) {
85+
t.Parallel()
86+
ctx, cancel := context.WithCancel(context.Background())
87+
defer cancel()
88+
89+
f, err := os.CreateTemp(t.TempDir(), "stdout")
90+
if err != nil {
91+
t.Fatal(err)
92+
}
93+
t.Cleanup(func() { _ = f.Close() })
94+
95+
cmd := exec.Command("true")
96+
s := CommandStage("true", cmd).(*commandStage)
97+
98+
p := New(tc.option(f))
99+
p.Add(s)
100+
if err := p.Start(ctx); err != nil {
101+
t.Fatalf("Start: %v", err)
102+
}
103+
stdoutAfterStart := s.cmd.Stdout
104+
t.Cleanup(func() { _ = p.Wait() })
105+
106+
gotFile, ok := stdoutAfterStart.(*os.File)
107+
if !ok {
108+
t.Fatalf("expected cmd.Stdout to be *os.File, got %T", stdoutAfterStart)
109+
}
110+
if gotFile != f {
111+
t.Errorf("expected cmd.Stdout to be the user-provided *os.File "+
112+
"(fd %d), got a different *os.File (fd %d). The fd-pass "+
113+
"fast path is broken; sendfile/zero-copy will not apply.",
114+
f.Fd(), gotFile.Fd())
115+
}
116+
})
117+
}
118+
}

0 commit comments

Comments
 (0)