Skip to content

Commit fa89a40

Browse files
committed
convert tests to testify
also drop one low-value test
1 parent a42eaea commit fa89a40

6 files changed

Lines changed: 71 additions & 136 deletions

pipe/close_responsibility_test.go

Lines changed: 26 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ import (
77
"strings"
88
"sync/atomic"
99
"testing"
10+
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
1013
)
1114

1215
// readCloseSpy records whether Close was called.
@@ -53,23 +56,15 @@ func TestGoStageHonorsCloseFlags(t *testing.T) {
5356
return err
5457
})
5558

56-
if err := s.Start(
59+
require.NoError(t, s.Start(
5760
context.Background(), StageOptions{},
5861
in, !tc.leaveIn,
5962
out, !tc.leaveOut,
60-
); err != nil {
61-
t.Fatalf("Start: %v", err)
62-
}
63-
if err := s.Wait(); err != nil {
64-
t.Fatalf("Wait: %v", err)
65-
}
66-
67-
if got, want := in.closed.Load(), !tc.leaveIn; got != want {
68-
t.Errorf("stdin closed = %v, want %v (closeStdin=%v)", got, want, !tc.leaveIn)
69-
}
70-
if got, want := out.closed.Load(), !tc.leaveOut; got != want {
71-
t.Errorf("stdout closed = %v, want %v (closeStdout=%v)", got, want, !tc.leaveOut)
72-
}
63+
))
64+
require.NoError(t, s.Wait())
65+
66+
assert.Equal(t, !tc.leaveIn, in.closed.Load(), "closeStdin=%v", !tc.leaveIn)
67+
assert.Equal(t, !tc.leaveOut, out.closed.Load(), "closeStdout=%v", !tc.leaveOut)
7368
})
7469
}
7570
}
@@ -79,21 +74,13 @@ func TestStagePanicsWhenOwnedStreamIsNotCloseable(t *testing.T) {
7974
return nil
8075
})
8176

82-
defer func() {
83-
r := recover()
84-
if r == nil {
85-
t.Fatal("expected Start to panic")
86-
}
87-
if !strings.Contains(r.(string), "does not implement io.Closer") {
88-
t.Fatalf("unexpected panic: %v", r)
89-
}
90-
}()
91-
92-
_ = s.Start(
93-
context.Background(), StageOptions{},
94-
strings.NewReader("not closeable"), true,
95-
nil, false,
96-
)
77+
assert.PanicsWithValue(t, "stage asked to close *strings.Reader, which does not implement io.Closer", func() {
78+
_ = s.Start(
79+
context.Background(), StageOptions{},
80+
strings.NewReader("not closeable"), true,
81+
nil, false,
82+
)
83+
})
9784
}
9885

9986
// TestCommandStageHonorsCloseStdin verifies that a command stage closes a
@@ -111,20 +98,14 @@ func TestCommandStageHonorsCloseStdin(t *testing.T) {
11198
cmd := exec.Command("true")
11299
s := CommandStage("true", cmd).(*commandStage)
113100

114-
if err := s.Start(
101+
require.NoError(t, s.Start(
115102
context.Background(), StageOptions{},
116103
in, !leave,
117104
nil, false,
118-
); err != nil {
119-
t.Fatalf("Start: %v", err)
120-
}
121-
if err := s.Wait(); err != nil {
122-
t.Fatalf("Wait: %v", err)
123-
}
124-
125-
if got, want := in.closed.Load(), !leave; got != want {
126-
t.Errorf("stdin closed = %v, want %v (closeStdin=%v)", got, want, !leave)
127-
}
105+
))
106+
require.NoError(t, s.Wait())
107+
108+
assert.Equal(t, !leave, in.closed.Load(), "closeStdin=%v", !leave)
128109
})
129110
}
130111
}
@@ -144,20 +125,14 @@ func TestCommandStageHonorsCloseStdout(t *testing.T) {
144125
cmd := exec.Command("true")
145126
s := CommandStage("true", cmd).(*commandStage)
146127

147-
if err := s.Start(
128+
require.NoError(t, s.Start(
148129
context.Background(), StageOptions{},
149130
nil, false,
150131
out, !leave,
151-
); err != nil {
152-
t.Fatalf("Start: %v", err)
153-
}
154-
if err := s.Wait(); err != nil {
155-
t.Fatalf("Wait: %v", err)
156-
}
157-
158-
if got, want := out.closed.Load(), !leave; got != want {
159-
t.Errorf("stdout closed = %v, want %v (closeStdout=%v)", got, want, !leave)
160-
}
132+
))
133+
require.NoError(t, s.Wait())
134+
135+
assert.Equal(t, !leave, out.closed.Load(), "closeStdout=%v", !leave)
161136
})
162137
}
163138
}

pipe/command_nil_panic_test.go

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"context"
88
"os/exec"
99
"testing"
10+
11+
"github.com/stretchr/testify/assert"
1012
)
1113

1214
func TestKillWithNilProcess(t *testing.T) {
@@ -17,31 +19,7 @@ func TestKillWithNilProcess(t *testing.T) {
1719
done: make(chan struct{}),
1820
}
1921

20-
defer func() {
21-
if r := recover(); r != nil {
22-
t.Fatalf("PANIC OCCURRED (bug not fixed): %v", r)
23-
}
24-
}()
25-
26-
stage.Kill(context.Canceled)
27-
28-
t.Log("SUCCESS: Kill() handled nil Process gracefully without panicking")
29-
}
30-
31-
func TestKillWithFailedStart(t *testing.T) {
32-
ctx := context.Background()
33-
34-
stage := Command("/this/path/does/not/exist/invalid_command_12345")
35-
36-
err := stage.Start(ctx, StageOptions{}, nil, false, nil, false)
37-
if err == nil {
38-
t.Fatal("Expected start to fail, but it succeeded")
39-
}
40-
41-
// At this point, if someone calls Kill (perhaps from a memory monitor
42-
// or other external component), it could panic if Process is nil
43-
44-
// Note: In the current implementation, Kill won't be called from the
45-
// context goroutine because Start failed. But external callers like
46-
// MemoryLimit could potentially call Kill on a failed stage.
22+
assert.NotPanics(t, func() {
23+
stage.Kill(context.Canceled)
24+
})
4725
}

pipe/command_starterror_test.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"testing"
99

1010
"github.com/github/go-pipe/v2/pipe"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
1113
)
1214

1315
// TestCommandStageStartFailureNoRace verifies that when `cmd.Start()`
@@ -23,9 +25,7 @@ func TestCommandStageStartFailureNoRace(t *testing.T) {
2325
var buf bytes.Buffer
2426
p := pipe.New(pipe.WithStdout(&buf))
2527
p.Add(pipe.CommandStage("nope", exec.Command("this-binary-does-not-exist-xyz123")))
26-
if err := p.Run(context.Background()); err == nil {
27-
t.Fatalf("expected error from non-existent command, got nil")
28-
}
28+
require.Error(t, p.Run(context.Background()))
2929
_ = buf.String()
3030
}
3131
}
@@ -54,10 +54,6 @@ func TestCommandStageStartFailureClosesLateClosers(t *testing.T) {
5454
w := &trackingWriteCloser{}
5555
p := pipe.New(pipe.WithStdoutCloser(w))
5656
p.Add(pipe.CommandStage("nope", exec.Command("this-binary-does-not-exist-xyz123")))
57-
if err := p.Run(context.Background()); err == nil {
58-
t.Fatalf("expected error from non-existent command, got nil")
59-
}
60-
if !w.closed.Load() {
61-
t.Fatalf("expected late closer to be closed after Start() failure")
62-
}
57+
require.Error(t, p.Run(context.Background()))
58+
assert.True(t, w.closed.Load(), "expected late closer to be closed after Start() failure")
6359
}

pipe/command_stdout_fastpath_test.go

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ import (
55
"os"
66
"os/exec"
77
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
811
)
912

1013
// TestCommandStageStdoutFastPath asserts that when a commandStage's stdout is
@@ -33,29 +36,24 @@ func TestCommandStageStdoutFastPath(t *testing.T) {
3336
defer cancel()
3437

3538
f, err := os.CreateTemp(t.TempDir(), "stdout")
36-
if err != nil {
37-
t.Fatal(err)
38-
}
39+
require.NoError(t, err)
3940
t.Cleanup(func() { _ = f.Close() })
4041

4142
cmd := exec.Command("true")
4243
s := CommandStage("true", cmd).(*commandStage)
4344

44-
if err := s.Start(ctx, StageOptions{}, nil, false, f, tc.closeStdout); err != nil {
45-
t.Fatalf("Start: %v", err)
46-
}
45+
require.NoError(t, s.Start(ctx, StageOptions{}, nil, false, f, tc.closeStdout))
4746
t.Cleanup(func() { _ = s.Wait() })
4847

4948
gotFile, ok := s.cmd.Stdout.(*os.File)
50-
if !ok {
51-
t.Fatalf("expected cmd.Stdout to be *os.File, got %T", s.cmd.Stdout)
52-
}
53-
if gotFile != f {
54-
t.Errorf("expected cmd.Stdout to be the user-provided *os.File "+
55-
"(fd %d), got a different *os.File (fd %d). The fd-pass "+
56-
"fast path is broken; sendfile/zero-copy will not apply.",
57-
f.Fd(), gotFile.Fd())
58-
}
49+
require.Truef(t, ok, "expected cmd.Stdout to be *os.File, got %T", s.cmd.Stdout)
50+
assert.Samef(
51+
t, f, gotFile,
52+
"expected cmd.Stdout to be the user-provided *os.File (fd %d), "+
53+
"got a different *os.File (fd %d). The fd-pass fast path is broken; "+
54+
"sendfile/zero-copy will not apply.",
55+
f.Fd(), gotFile.Fd(),
56+
)
5957
})
6058
}
6159
}
@@ -85,32 +83,27 @@ func TestCommandStageStdoutFastPathThroughPipeline(t *testing.T) {
8583
defer cancel()
8684

8785
f, err := os.CreateTemp(t.TempDir(), "stdout")
88-
if err != nil {
89-
t.Fatal(err)
90-
}
86+
require.NoError(t, err)
9187
t.Cleanup(func() { _ = f.Close() })
9288

9389
cmd := exec.Command("true")
9490
s := CommandStage("true", cmd).(*commandStage)
9591

9692
p := New(tc.option(f))
9793
p.Add(s)
98-
if err := p.Start(ctx); err != nil {
99-
t.Fatalf("Start: %v", err)
100-
}
94+
require.NoError(t, p.Start(ctx))
10195
stdoutAfterStart := s.cmd.Stdout
10296
t.Cleanup(func() { _ = p.Wait() })
10397

10498
gotFile, ok := stdoutAfterStart.(*os.File)
105-
if !ok {
106-
t.Fatalf("expected cmd.Stdout to be *os.File, got %T", stdoutAfterStart)
107-
}
108-
if gotFile != f {
109-
t.Errorf("expected cmd.Stdout to be the user-provided *os.File "+
110-
"(fd %d), got a different *os.File (fd %d). The fd-pass "+
111-
"fast path is broken; sendfile/zero-copy will not apply.",
112-
f.Fd(), gotFile.Fd())
113-
}
99+
require.Truef(t, ok, "expected cmd.Stdout to be *os.File, got %T", stdoutAfterStart)
100+
assert.Samef(
101+
t, f, gotFile,
102+
"expected cmd.Stdout to be the user-provided *os.File (fd %d), "+
103+
"got a different *os.File (fd %d). The fd-pass fast path is broken; "+
104+
"sendfile/zero-copy will not apply.",
105+
f.Fd(), gotFile.Fd(),
106+
)
114107
})
115108
}
116109
}

pipe/function_panic_test.go

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@ import (
55
"io"
66
"os"
77
"os/exec"
8-
"strings"
98
"testing"
109
"time"
1110

1211
"github.com/github/go-pipe/v2/pipe"
12+
"github.com/stretchr/testify/assert"
13+
"github.com/stretchr/testify/require"
1314
)
1415

1516
const panicChildEnv = "GO_PIPE_FUNCTION_PANIC_CHILD"
@@ -32,15 +33,10 @@ func TestFunctionPanicWithoutHandlerPropagates(t *testing.T) {
3233
out, err := cmd.CombinedOutput()
3334
output := string(out)
3435

35-
if err == nil {
36-
t.Fatalf("expected subprocess to crash from a propagated panic, but it exited 0\noutput:\n%s", output)
37-
}
38-
if strings.Contains(output, "SURVIVED") {
39-
t.Fatalf("panic was swallowed: Run returned instead of propagating\noutput:\n%s", output)
40-
}
41-
if !strings.Contains(output, "panic:") || !strings.Contains(output, panicSentinel) {
42-
t.Fatalf("expected a propagated panic mentioning %q, got:\n%s", panicSentinel, output)
43-
}
36+
require.Errorf(t, err, "expected subprocess to crash from a propagated panic, but it exited 0\noutput:\n%s", output)
37+
assert.NotContains(t, output, "SURVIVED", "panic was swallowed: Run returned instead of propagating")
38+
assert.Contains(t, output, "panic:")
39+
assert.Contains(t, output, panicSentinel)
4440
}
4541

4642
// runPanicChild runs a pipeline whose only stage is a `Function` that panics,

pipe/nop_closer_test.go

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ import (
55
"context"
66
"io"
77
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
811
)
912

1013
// TestGoStageReceivesConcreteWriterToStdin verifies that a Function stage
@@ -22,14 +25,8 @@ func TestGoStageReceivesConcreteWriterToStdin(t *testing.T) {
2225
return err
2326
}))
2427

25-
if err := p.Run(context.Background()); err != nil {
26-
t.Fatalf("Run: %v", err)
27-
}
28+
require.NoError(t, p.Run(context.Background()))
2829

29-
if got != io.Reader(src) {
30-
t.Fatalf("StageFunc stdin = %T %p, want *bytes.Reader %p", got, got, src)
31-
}
32-
if _, ok := got.(io.WriterTo); !ok {
33-
t.Fatalf("stdin %T does not expose io.WriterTo fast path", got)
34-
}
30+
assert.Same(t, src, got)
31+
assert.Implements(t, (*io.WriterTo)(nil), got)
3532
}

0 commit comments

Comments
 (0)