From c6989870b00632cacb9678b68d41fdaca3832d76 Mon Sep 17 00:00:00 2001 From: Joshua Gilman Date: Sun, 8 Sep 2024 13:27:08 -0400 Subject: [PATCH] chore: refactors forge/cli tests with assert --- forge/cli/go.mod | 5 +- forge/cli/pkg/earthfile/earthfile_test.go | 46 +++++----------- forge/cli/pkg/earthfile/scan_test.go | 37 ++++--------- forge/cli/pkg/earthly/earthly_test.go | 55 ++++++------------- forge/cli/pkg/secrets/providers/aws_test.go | 52 ++++++------------ forge/cli/pkg/secrets/providers/local_test.go | 22 +++----- tools/pkg/testutils/helpers.go | 7 +++ 7 files changed, 77 insertions(+), 147 deletions(-) diff --git a/forge/cli/go.mod b/forge/cli/go.mod index b24060da..c15a9a20 100644 --- a/forge/cli/go.mod +++ b/forge/cli/go.mod @@ -11,8 +11,10 @@ require ( github.com/charmbracelet/log v0.4.0 github.com/earthly/earthly/ast v0.0.2-0.20240228223838-42e8ca204e8a github.com/input-output-hk/catalyst-forge/blueprint v0.0.0 + github.com/input-output-hk/catalyst-forge/tools v0.0.0 github.com/rogpeppe/go-internal v1.12.1-0.20240709150035-ccf4b4329d21 github.com/spf13/afero v1.11.0 + github.com/stretchr/testify v1.9.0 ) require ( @@ -33,15 +35,16 @@ require ( github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/charmbracelet/lipgloss v0.10.0 // indirect github.com/cockroachdb/apd/v3 v3.2.1 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/go-logfmt/logfmt v0.6.0 // indirect github.com/google/uuid v1.6.0 // indirect - github.com/input-output-hk/catalyst-forge/tools v0.0.0 // indirect github.com/lucasb-eyer/go-colorful v1.2.0 // indirect github.com/mattn/go-isatty v0.0.18 // indirect github.com/mattn/go-runewidth v0.0.15 // indirect github.com/muesli/reflow v0.3.0 // indirect github.com/muesli/termenv v0.15.2 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/rivo/uniseg v0.4.7 // indirect golang.org/x/exp v0.0.0-20231006140011-7918f672742d // indirect golang.org/x/mod v0.20.0 // indirect diff --git a/forge/cli/pkg/earthfile/earthfile_test.go b/forge/cli/pkg/earthfile/earthfile_test.go index 6e309d59..824ba7d9 100644 --- a/forge/cli/pkg/earthfile/earthfile_test.go +++ b/forge/cli/pkg/earthfile/earthfile_test.go @@ -5,7 +5,8 @@ import ( "testing" "github.com/earthly/earthly/ast/spec" - "github.com/input-output-hk/catalyst-forge/forge/cli/internal/testutils/mocks" + "github.com/input-output-hk/catalyst-forge/tools/pkg/testutils" + "github.com/stretchr/testify/assert" ) func TestEarthfileTargets(t *testing.T) { @@ -19,17 +20,9 @@ func TestEarthfileTargets(t *testing.T) { } targets := earthfile.Targets() - if len(targets) != 2 { - t.Errorf("expected 2 targets, got %d", len(targets)) - } - - if targets[0] != "target1" { - t.Errorf("expected target1, got %s", targets[0]) - } - - if targets[1] != "target2" { - t.Errorf("expected target2, got %s", targets[1]) - } + assert.Equal(t, 2, len(targets), "expected 2 targets") + assert.Equal(t, "target1", targets[0], "expected target1") + assert.Equal(t, "target2", targets[1], "expected target2") } func TestEarthfileFilterTargets(t *testing.T) { @@ -46,43 +39,32 @@ func TestEarthfileFilterTargets(t *testing.T) { return target == "target1" }) - if len(targets) != 1 { - t.Errorf("expected 1 target, got %d", len(targets)) - } - - if targets[0] != "target1" { - t.Errorf("expected target1, got %s", targets[0]) - } + assert.Equal(t, 1, len(targets), "expected 1 target") + assert.Equal(t, "target1", targets[0], "expected target1") } func TestParseEarthfile(t *testing.T) { tests := []struct { - name string - content string - hasError bool + name string + content string + expectErr bool }{ { - name: "valid earthfile", - hasError: false, + name: "valid earthfile", content: ` VERSION 0.7 foo: LET foo = bar `, + expectErr: false, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - _, err := ParseEarthfile(context.Background(), mocks.NewMockFileSeeker(test.content)) - if test.hasError && err == nil { - t.Error("expected error, got nil") - } - - if !test.hasError && err != nil { - t.Errorf("expected no error, got %v", err) - } + _, err := ParseEarthfile(context.Background(), NewMockFileSeeker(test.content)) + testutils.AssertError(t, err, test.expectErr, "") }) } } diff --git a/forge/cli/pkg/earthfile/scan_test.go b/forge/cli/pkg/earthfile/scan_test.go index b16f77b6..f8de50f5 100644 --- a/forge/cli/pkg/earthfile/scan_test.go +++ b/forge/cli/pkg/earthfile/scan_test.go @@ -9,8 +9,11 @@ import ( "log/slog" + "github.com/input-output-hk/catalyst-forge/tools/pkg/testutils" "github.com/input-output-hk/catalyst-forge/tools/pkg/walker" "github.com/input-output-hk/catalyst-forge/tools/pkg/walker/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type MockFileSeeker struct { @@ -39,11 +42,11 @@ func NewMockFileSeeker(s string) MockFileSeeker { func TestScanEarthfiles(t *testing.T) { tests := []struct { - callbackErr error - walkErr error + name string files map[string]string expectedResult map[string][]string - name string + callbackErr error + walkErr error }{ { name: "one earthfile", @@ -134,36 +137,18 @@ foo1: return tt.walkErr }, } - result, err := ScanEarthfiles("/", walker, slog.New(slog.NewTextHandler(io.Discard, nil))) - fmt.Printf("result: %v\n", result) - - if tt.callbackErr != nil && err == nil { - t.Error("expected error, got nil") - } else if tt.walkErr != nil && err == nil { - t.Error("expected error, got nil") - } else if tt.callbackErr == nil && tt.walkErr == nil && err != nil { - t.Errorf("expected no error, got %v", err) - } else { - if err != nil { - return - } - } - if len(result) != len(tt.expectedResult) { - t.Errorf("expected %d earthfiles, got %d", len(tt.expectedResult), len(result)) + result, err := ScanEarthfiles("/", walker, slog.New(slog.NewTextHandler(io.Discard, nil))) + if testutils.AssertError(t, err, tt.callbackErr != nil || tt.walkErr != nil, "") { return } + require.Equal(t, len(tt.expectedResult), len(result)) for path, targets := range tt.expectedResult { - if len(result[path].Targets()) != len(targets) { - t.Errorf("expected %d targets for %s, got %d", len(targets), path, len(result[path].Targets())) - return - } + require.Equal(t, len(targets), len(result[path].Targets())) for i, target := range targets { - if result[path].Targets()[i] != target { - t.Errorf("expected target %s at index %d, got %s", target, i, result[path].Targets()[i]) - } + assert.Equal(t, target, result[path].Targets()[i]) } } }) diff --git a/forge/cli/pkg/earthly/earthly_test.go b/forge/cli/pkg/earthly/earthly_test.go index 07f3e245..9d2b43ef 100644 --- a/forge/cli/pkg/earthly/earthly_test.go +++ b/forge/cli/pkg/earthly/earthly_test.go @@ -3,17 +3,15 @@ package earthly import ( "fmt" "log/slog" - "maps" - "reflect" - "slices" "testing" "github.com/input-output-hk/catalyst-forge/blueprint/pkg/utils" "github.com/input-output-hk/catalyst-forge/blueprint/schema" - "github.com/input-output-hk/catalyst-forge/forge/cli/internal/testutils" emocks "github.com/input-output-hk/catalyst-forge/forge/cli/pkg/executor/mocks" "github.com/input-output-hk/catalyst-forge/forge/cli/pkg/secrets" smocks "github.com/input-output-hk/catalyst-forge/forge/cli/pkg/secrets/mocks" + "github.com/input-output-hk/catalyst-forge/tools/pkg/testutils" + "github.com/stretchr/testify/assert" ) func TestEarthlyExecutorRun(t *testing.T) { @@ -108,19 +106,12 @@ Artifact foo output as bar`), nil tt.earthlyExec.executor = &tt.mockExec got, err := tt.earthlyExec.Run() - if tt.expectErr && err == nil { - t.Errorf("expected error, got nil") - } else if !tt.expectErr && err != nil { - t.Errorf("unexpected error: %v", err) - } - - if len(tt.mockExec.ExecuteCalls()) != tt.expectCalls { - t.Errorf("expected %d calls to Execute, got %d", tt.expectCalls, len(tt.mockExec.ExecuteCalls())) + if testutils.AssertError(t, err, tt.expectErr, "") { + return } - if !reflect.DeepEqual(got, tt.expect) { - t.Errorf("expected %v, got %v", tt.expect, got) - } + assert.Equal(t, len(tt.mockExec.ExecuteCalls()), tt.expectCalls) + assert.Equal(t, tt.expect, got) }) } } @@ -208,9 +199,7 @@ func TestEarthlyExecutor_buildArguments(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := tt.e.buildArguments(tt.platform) - if !slices.Equal(got, tt.expect) { - t.Errorf("expected %v, got %v", tt.expect, got) - } + assert.Equal(t, tt.expect, got) }) } } @@ -222,7 +211,7 @@ func TestEarthlyExecutor_buildSecrets(t *testing.T) { secrets []schema.Secret expect []EarthlySecret expectErr bool - expectedErr error + expectedErr string }{ { name: "simple", @@ -247,7 +236,7 @@ func TestEarthlyExecutor_buildSecrets(t *testing.T) { }, }, expectErr: false, - expectedErr: nil, + expectedErr: "", }, { name: "key does not exist", @@ -267,7 +256,7 @@ func TestEarthlyExecutor_buildSecrets(t *testing.T) { }, expect: nil, expectErr: true, - expectedErr: fmt.Errorf("secret key not found in secret values: key1"), + expectedErr: "secret key not found in secret values: key1", }, { name: "invalid JSON", @@ -285,7 +274,7 @@ func TestEarthlyExecutor_buildSecrets(t *testing.T) { }, expect: nil, expectErr: true, - expectedErr: fmt.Errorf("unable to unmarshal secret value: invalid character 'i' looking for beginning of value"), + expectedErr: "unable to unmarshal secret value: invalid character 'i' looking for beginning of value", }, { name: "secret provider does not exist", @@ -303,7 +292,7 @@ func TestEarthlyExecutor_buildSecrets(t *testing.T) { }, expect: nil, expectErr: true, - expectedErr: fmt.Errorf("unable to create new secret client: unknown secret provider: bad"), + expectedErr: "unable to create new secret client: unknown secret provider: bad", }, { name: "secret provider error", @@ -321,7 +310,7 @@ func TestEarthlyExecutor_buildSecrets(t *testing.T) { }, expect: nil, expectErr: true, - expectedErr: fmt.Errorf("unable to get secret path from provider: mock"), + expectedErr: "unable to get secret path from provider: mock", }, } @@ -337,17 +326,11 @@ func TestEarthlyExecutor_buildSecrets(t *testing.T) { executor.secrets = tt.secrets got, err := executor.buildSecrets() - ret, err := testutils.CheckError(t, err, tt.expectErr, tt.expectedErr) - if err != nil { - t.Error(err) - return - } else if ret { + if testutils.AssertError(t, err, tt.expectErr, tt.expectedErr) { return } - if !slices.Equal(got, tt.expect) { - t.Errorf("expected %v, got %v", tt.expect, got) - } + assert.Equal(t, tt.expect, got) }) } } @@ -385,12 +368,8 @@ Artifact foo output as bar`, for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := parseResult(tt.output) - if !maps.Equal(got.Images, tt.expect.Images) { - t.Errorf("expected %v, got %v", tt.expect.Images, got.Images) - } - if !maps.Equal(got.Artifacts, tt.expect.Artifacts) { - t.Errorf("expected %v, got %v", tt.expect.Artifacts, got.Artifacts) - } + assert.Equal(t, tt.expect, got) + assert.Equal(t, tt.expect.Images, got.Images) }) } } diff --git a/forge/cli/pkg/secrets/providers/aws_test.go b/forge/cli/pkg/secrets/providers/aws_test.go index f13b6869..1e9d6cc3 100644 --- a/forge/cli/pkg/secrets/providers/aws_test.go +++ b/forge/cli/pkg/secrets/providers/aws_test.go @@ -7,7 +7,9 @@ import ( "github.com/aws/aws-sdk-go-v2/service/secretsmanager" "github.com/aws/aws-sdk-go/aws" - "github.com/input-output-hk/catalyst-forge/forge/cli/internal/testutils" + "github.com/input-output-hk/catalyst-forge/tools/pkg/testutils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestAWSClientGet(t *testing.T) { @@ -17,7 +19,7 @@ func TestAWSClientGet(t *testing.T) { mock SecretsManagerClientMock expect string expectErr bool - expectedErr error + expectedErr string cond func(*SecretsManagerClientMock) error }{ { @@ -32,7 +34,7 @@ func TestAWSClientGet(t *testing.T) { }, expect: "secret", expectErr: false, - expectedErr: nil, + expectedErr: "", cond: func(m *SecretsManagerClientMock) error { if len(m.calls.GetSecretValue) != 1 { return fmt.Errorf("expected GetSecretValue to be called once, got %d", len(m.calls.GetSecretValue)) @@ -54,7 +56,7 @@ func TestAWSClientGet(t *testing.T) { }, expect: "", expectErr: true, - expectedErr: fmt.Errorf("unable to get secret: error"), + expectedErr: "unable to get secret: error", }, } @@ -68,24 +70,14 @@ func TestAWSClientGet(t *testing.T) { got, err := client.Get(tt.path) - ret, err := testutils.CheckError(t, err, tt.expectErr, tt.expectedErr) - if err != nil { - t.Error(err) - return - } else if ret { + if testutils.AssertError(t, err, tt.expectErr, tt.expectedErr) { return } if tt.cond != nil { - if err := tt.cond(&tt.mock); err != nil { - t.Error(err) - return - } - } - - if got != tt.expect { - t.Errorf("expected: %s, got: %s", tt.expect, got) + require.NoError(t, tt.cond(&tt.mock)) } + assert.Equal(t, tt.expect, got) }) } } @@ -96,7 +88,7 @@ func TestAWSClientSet(t *testing.T) { mock SecretsManagerClientMock expect string expectErr bool - expectedErr error + expectedErr string cond func(*SecretsManagerClientMock) error }{ { @@ -110,7 +102,7 @@ func TestAWSClientSet(t *testing.T) { }, expect: "version", expectErr: false, - expectedErr: nil, + expectedErr: "", cond: func(m *SecretsManagerClientMock) error { if len(m.calls.CreateSecret) != 1 { return fmt.Errorf("expected CreateSecret to be called once, got %d", len(m.calls.CreateSecret)) @@ -137,7 +129,7 @@ func TestAWSClientSet(t *testing.T) { }, expect: "version", expectErr: false, - expectedErr: nil, + expectedErr: "", cond: func(m *SecretsManagerClientMock) error { if len(m.calls.CreateSecret) != 1 { return fmt.Errorf("expected CreateSecret to be called once, got %d", len(m.calls.CreateSecret)) @@ -167,7 +159,7 @@ func TestAWSClientSet(t *testing.T) { }, expect: "", expectErr: true, - expectedErr: fmt.Errorf("unable to set secret: error"), + expectedErr: "unable to set secret: error", }, { name: "error putting secret value", @@ -181,7 +173,7 @@ func TestAWSClientSet(t *testing.T) { }, expect: "", expectErr: true, - expectedErr: fmt.Errorf("unable to set secret: error"), + expectedErr: "unable to set secret: error", }, } @@ -195,24 +187,14 @@ func TestAWSClientSet(t *testing.T) { got, err := client.Set("path", "value") - ret, err := testutils.CheckError(t, err, tt.expectErr, tt.expectedErr) - if err != nil { - t.Error(err) - return - } else if ret { + if testutils.AssertError(t, err, tt.expectErr, tt.expectedErr) { return } if tt.cond != nil { - if err := tt.cond(&tt.mock); err != nil { - t.Error(err) - return - } - } - - if got != tt.expect { - t.Errorf("expected: %s, got: %s", tt.expect, got) + assert.NoError(t, tt.cond(&tt.mock)) } + assert.Equal(t, tt.expect, got) }) } } diff --git a/forge/cli/pkg/secrets/providers/local_test.go b/forge/cli/pkg/secrets/providers/local_test.go index fb1b0cf1..09a15099 100644 --- a/forge/cli/pkg/secrets/providers/local_test.go +++ b/forge/cli/pkg/secrets/providers/local_test.go @@ -1,11 +1,11 @@ package providers import ( - "fmt" "testing" - "github.com/input-output-hk/catalyst-forge/forge/cli/internal/testutils" + "github.com/input-output-hk/catalyst-forge/tools/pkg/testutils" "github.com/spf13/afero" + "github.com/stretchr/testify/assert" ) func TestLocalClientGet(t *testing.T) { @@ -15,7 +15,7 @@ func TestLocalClientGet(t *testing.T) { files map[string]string expect string expectErr bool - expectedErr error + expectedErr string }{ { name: "simple", @@ -25,7 +25,7 @@ func TestLocalClientGet(t *testing.T) { }, expect: "secret", expectErr: false, - expectedErr: nil, + expectedErr: "", }, { name: "file not found", @@ -35,7 +35,7 @@ func TestLocalClientGet(t *testing.T) { }, expect: "", expectErr: true, - expectedErr: fmt.Errorf("open foo: file does not exist"), + expectedErr: "open foo: file does not exist", }, } @@ -52,18 +52,10 @@ func TestLocalClientGet(t *testing.T) { } got, err := client.Get(tt.key) - - ret, err := testutils.CheckError(t, err, tt.expectErr, tt.expectedErr) - if err != nil { - t.Error(err) - return - } else if ret { + if testutils.AssertError(t, err, tt.expectErr, tt.expectedErr) { return } - - if got != tt.expect { - t.Errorf("expected: %s, got: %s", tt.expect, got) - } + assert.Equal(t, tt.expect, got) }) } } diff --git a/tools/pkg/testutils/helpers.go b/tools/pkg/testutils/helpers.go index d8a061ea..76606e12 100644 --- a/tools/pkg/testutils/helpers.go +++ b/tools/pkg/testutils/helpers.go @@ -2,6 +2,8 @@ package testutils import ( + "io" + "log/slog" "path/filepath" "testing" @@ -32,6 +34,11 @@ func AssertError(t *testing.T, err error, isExpected bool, expectedErr string) b return false } +// NewNoopLogger creates a new logger that discards all logs. +func NewNoopLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(io.Discard, nil)) +} + // SetupFS sets up the filesystem with the given files. func SetupFS(t *testing.T, fs afero.Fs, files map[string]string) { for path, content := range files {