From df0197382432ed738616566d23f5832a4cb58184 Mon Sep 17 00:00:00 2001 From: yxxhero <11087727+yxxhero@users.noreply.github.com> Date: Tue, 13 Jun 2023 13:44:32 +0800 Subject: [PATCH] cleanup: remove panic in testutil (#890) * cleanup: remove panic in testutil Signed-off-by: yxxhero --- pkg/app/app_list_test.go | 6 ++++-- pkg/app/app_test.go | 11 ++++++++--- pkg/app/formatters_test.go | 11 +++++++++-- pkg/testutil/testutil.go | 21 +++++++++++---------- pkg/testutil/testutil_test.go | 5 ++++- 5 files changed, 36 insertions(+), 18 deletions(-) diff --git a/pkg/app/app_list_test.go b/pkg/app/app_list_test.go index ffb53981..08a5cae4 100644 --- a/pkg/app/app_list_test.go +++ b/pkg/app/app_list_test.go @@ -136,9 +136,10 @@ releases: } var listErr error - out := testutil.CaptureStdout(func() { + out, err := testutil.CaptureStdout(func() { listErr = app.ListReleases(cfg) }) + assert.NoError(t, err) var gotErr string if listErr != nil { @@ -277,10 +278,11 @@ releases: expectNoCallsToHelm(app) - out := testutil.CaptureStdout(func() { + out, err := testutil.CaptureStdout(func() { err := app.ListReleases(cfg) assert.Nil(t, err) }) + assert.NoError(t, err) expected := `[{"name":"myrelease1","namespace":"testNamespace","enabled":true,"installed":false,"labels":"id:myrelease1","chart":"mychart1","version":""},{"name":"myrelease2","namespace":"testNamespace","enabled":false,"installed":true,"labels":"","chart":"mychart1","version":""},{"name":"myrelease3","namespace":"testNamespace","enabled":true,"installed":true,"labels":"","chart":"mychart1","version":""},{"name":"myrelease4","namespace":"testNamespace","enabled":true,"installed":true,"labels":"id:myrelease1","chart":"mychart1","version":""}] ` diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 8c5292b5..fd14d4d7 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -4052,10 +4052,13 @@ releases: expectNoCallsToHelm(app) - out := testutil.CaptureStdout(func() { + out, err := testutil.CaptureStdout(func() { err := app.PrintState(configImpl{}) assert.Nil(t, err) }) + + assert.NoError(t, err) + assert.True(t, strings.Count(out, "---") == 1, "state should contain '---' yaml doc separator:\n%s\n", out) assert.True(t, strings.Contains(out, "helmfile.yaml"), @@ -4098,10 +4101,11 @@ releases: expectNoCallsToHelm(app) - out := testutil.CaptureStdout(func() { + out, err := testutil.CaptureStdout(func() { err := app.PrintState(configImpl{}) assert.Nil(t, err) }) + assert.NoError(t, err) assert.True(t, strings.Count(out, "---") == 2, "state should contain '---' yaml doc separators:\n%s\n", out) assert.True(t, strings.Contains(out, "second.yaml"), @@ -4158,10 +4162,11 @@ releases: expectNoCallsToHelm(app) - out := testutil.CaptureStdout(func() { + out, err := testutil.CaptureStdout(func() { err := app.ListReleases(configImpl{}) assert.Nil(t, err) }) + assert.NoError(t, err) expected := `NAME NAMESPACE ENABLED INSTALLED LABELS CHART VERSION myrelease1 testNamespace true false common:label,id:myrelease1 mychart1 diff --git a/pkg/app/formatters_test.go b/pkg/app/formatters_test.go index fe40be18..e6840ee1 100644 --- a/pkg/app/formatters_test.go +++ b/pkg/app/formatters_test.go @@ -4,6 +4,8 @@ import ( "os" "testing" + "github.com/stretchr/testify/assert" + "github.com/helmfile/helmfile/pkg/testutil" ) @@ -36,9 +38,12 @@ func TestFormatAsTable(t *testing.T) { t.Errorf("error reading %s: %v", tableoutput, err) } - result := testutil.CaptureStdout(func() { + result, err := testutil.CaptureStdout(func() { FormatAsTable(h) }) + + assert.NoError(t, err) + if result != string(expectd) { t.Errorf("FormatAsTable() = %v, want %v", result, string(expectd)) } @@ -70,10 +75,12 @@ func TestFormatAsJson(t *testing.T) { if err != nil { t.Errorf("error reading %s: %v", output, err) } - result := testutil.CaptureStdout(func() { + result, err := testutil.CaptureStdout(func() { FormatAsJson(h) }) + assert.NoError(t, err) + if result != string(expectd) { t.Errorf("FormatAsJson() = %v, want %v", result, string(expectd)) } diff --git a/pkg/testutil/testutil.go b/pkg/testutil/testutil.go index 10b49600..e5bcea08 100644 --- a/pkg/testutil/testutil.go +++ b/pkg/testutil/testutil.go @@ -9,10 +9,10 @@ import ( ) // CaptureStdout is a helper function to capture stdout. -func CaptureStdout(f func()) string { +func CaptureStdout(f func()) (string, error) { reader, writer, err := os.Pipe() if err != nil { - panic(err) + return "", err } stdout := os.Stdout defer func() { @@ -21,20 +21,21 @@ func CaptureStdout(f func()) string { }() os.Stdout = writer log.SetOutput(writer) - out := make(chan string) + out := make(chan string, 1) wg := new(sync.WaitGroup) wg.Add(1) + var ioCopyErr error go func() { var buf bytes.Buffer - wg.Done() - _, err = io.Copy(&buf, reader) - if err != nil { - panic(err) - } + defer wg.Done() + _, ioCopyErr = io.Copy(&buf, reader) out <- buf.String() }() - wg.Wait() f() _ = writer.Close() - return <-out + wg.Wait() + if ioCopyErr != nil { + return "", ioCopyErr + } + return <-out, nil } diff --git a/pkg/testutil/testutil_test.go b/pkg/testutil/testutil_test.go index 06460756..6bc882b6 100644 --- a/pkg/testutil/testutil_test.go +++ b/pkg/testutil/testutil_test.go @@ -3,6 +3,8 @@ package testutil import ( "fmt" "testing" + + "github.com/stretchr/testify/assert" ) // TestCaptureStdout tests the CaptureStdout function. @@ -30,9 +32,10 @@ func TestCaptureStdout(t *testing.T) { } for _, test := range tests { - result := CaptureStdout(func() { + result, err := CaptureStdout(func() { fmt.Print(test.output) }) + assert.NoError(t, err) if result != test.expected { t.Errorf("CaptureStdout() = %v, want %v", result, test.expected) }