From cd15a65383605634d5feb02d01f4731960c76254 Mon Sep 17 00:00:00 2001 From: Jay Pipes Date: Tue, 8 Aug 2023 11:14:47 -0400 Subject: [PATCH] fix: call t.Error() from test spec Brings in gdt@v1.1.1 and ensures that the test units/specs call `testing.T.Error()` instead of relying on the `Scenario.Run()` to do that. Also adds a custom YAML unmarshaler for the `Expect` struct and adds better parse-time errors for the `matches` field as requested in Issue 8. Addresses Issue gdt-dev/gdt#8 Signed-off-by: Jay Pipes --- errors.go | 18 +++ eval.go | 11 ++ eval_test.go | 32 ---- go.mod | 2 +- go.sum | 4 +- parse.go | 138 +++++++++++++----- parse_test.go | 17 ++- testdata/parse/fail/bad-matches-empty.yaml | 7 + .../fail/unknown-context-in-defaults.yaml | 9 -- testdata/parse/fail/unknown-context.yaml | 6 - 10 files changed, 158 insertions(+), 86 deletions(-) create mode 100644 testdata/parse/fail/bad-matches-empty.yaml delete mode 100644 testdata/parse/fail/unknown-context-in-defaults.yaml delete mode 100644 testdata/parse/fail/unknown-context.yaml diff --git a/errors.go b/errors.go index 969cf3d..8c1ee6b 100644 --- a/errors.go +++ b/errors.go @@ -13,6 +13,15 @@ import ( ) var ( + // ErrExpectedMapOrYAMLString is returned when a field that can contain a + // map[string]interface{} or an embedded YAML string did not contain either + // of those things. + // TODO(jaypipes): Move to gdt core? + ErrExpectedMapOrYAMLString = fmt.Errorf( + "%w: expected either map[string]interface{} "+ + "or a string with embedded YAML", + gdterrors.ErrParse, + ) // ErrMoreThanOneShortcut is returned when the test author included // more than one shortcut (e.g. `kube.create` or `kube.apply`) in the same // test spec. @@ -119,6 +128,15 @@ var ( ) ) +// ExpectedMapOrYAMLStringAt returns ErrExpectedMapOrYAMLString for a given +// YAML node +func ExpectedMapOrYAMLStringAt(node *yaml.Node) error { + return fmt.Errorf( + "%w at line %d, column %d", + ErrExpectedMapOrYAMLString, node.Line, node.Column, + ) +} + // KubeConfigNotFound returns ErrKubeConfigNotFound for a given filepath func KubeConfigNotFound(path string) error { return fmt.Errorf("%w: %s", ErrKubeConfigNotFound, path) diff --git a/eval.go b/eval.go index 48f58d7..7104bf2 100644 --- a/eval.go +++ b/eval.go @@ -16,6 +16,7 @@ import ( "time" backoff "github.com/cenkalti/backoff/v4" + gdtcontext "github.com/gdt-dev/gdt/context" "github.com/gdt-dev/gdt/debug" gdterrors "github.com/gdt-dev/gdt/errors" "github.com/gdt-dev/gdt/parse" @@ -60,6 +61,16 @@ func (s *Spec) Eval(ctx context.Context, t *testing.T) *result.Result { if s.Kube.Apply != "" { res = s.apply(ctx, t, c) } + for _, failure := range res.Failures() { + if gdtcontext.TimedOut(ctx, failure) { + to := s.Timeout + if to != nil && !to.Expected { + t.Error(gdterrors.TimeoutExceeded(to.After, failure)) + } + } else { + t.Error(failure) + } + } }) return res } diff --git a/eval_test.go b/eval_test.go index aedb3f5..58f6484 100644 --- a/eval_test.go +++ b/eval_test.go @@ -5,7 +5,6 @@ package kube_test import ( - "context" "os" "path/filepath" "testing" @@ -13,40 +12,9 @@ import ( "github.com/gdt-dev/gdt" gdtcontext "github.com/gdt-dev/gdt/context" kindfix "github.com/gdt-dev/kube/fixtures/kind" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestUnknownKubeContextInSpec(t *testing.T) { - require := require.New(t) - assert := assert.New(t) - - fp := filepath.Join("testdata", "parse", "fail", "unknown-context.yaml") - - s, err := gdt.From(fp) - require.Nil(err) - require.NotNil(s) - - err = s.Run(context.TODO(), t) - assert.NotNil(err) - assert.ErrorContains(err, "context \"unknownctx\" does not exist") -} - -func TestUnknownKubeContextInDefaults(t *testing.T) { - require := require.New(t) - assert := assert.New(t) - - fp := filepath.Join("testdata", "parse", "fail", "unknown-context-in-defaults.yaml") - - s, err := gdt.From(fp) - require.Nil(err) - require.NotNil(s) - - err = s.Run(context.TODO(), t) - require.NotNil(err) - assert.ErrorContains(err, "context \"unknownctx\" does not exist") -} - func TestListPodsEmpty(t *testing.T) { skipIfKind(t) require := require.New(t) diff --git a/go.mod b/go.mod index ac43620..bc1fbe9 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.19 require ( github.com/cenkalti/backoff/v4 v4.2.1 - github.com/gdt-dev/gdt v1.1.0 + github.com/gdt-dev/gdt v1.1.1 github.com/samber/lo v1.38.1 github.com/stretchr/testify v1.8.4 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index 7d72a9f..224bdcc 100644 --- a/go.sum +++ b/go.sum @@ -65,8 +65,8 @@ github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7 github.com/evanphx/json-patch v4.12.0+incompatible h1:4onqiflcdA9EOZ4RxV643DvftH5pOlLGNtQ5lPWQu84= github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww= github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4= -github.com/gdt-dev/gdt v1.1.0 h1:+eFYFSibOYCTFKqoACx2CefAbErmBmFWXG7kDioR5do= -github.com/gdt-dev/gdt v1.1.0/go.mod h1:StnyGjC/67u59La2u6fh3HwW9MmodVhKdXcLlkgvNSY= +github.com/gdt-dev/gdt v1.1.1 h1:863WjQr2Oa+1eVKJspw1SRqW72S0Y3UydN0j8WwPYJU= +github.com/gdt-dev/gdt v1.1.1/go.mod h1:StnyGjC/67u59La2u6fh3HwW9MmodVhKdXcLlkgvNSY= github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= diff --git a/parse.go b/parse.go index 6bf8cce..eee74a6 100644 --- a/parse.go +++ b/parse.go @@ -8,6 +8,7 @@ import ( "os" "strings" + gdtjson "github.com/gdt-dev/gdt/assertion/json" "github.com/gdt-dev/gdt/errors" gdttypes "github.com/gdt-dev/gdt/types" "github.com/samber/lo" @@ -84,6 +85,109 @@ func (s *Spec) UnmarshalYAML(node *yaml.Node) error { return nil } +func (e *Expect) UnmarshalYAML(node *yaml.Node) error { + if node.Kind != yaml.MappingNode { + return errors.ExpectedMapAt(node) + } + // maps/structs are stored in a top-level Node.Content field which is a + // concatenated slice of Node pointers in pairs of key/values. + for i := 0; i < len(node.Content); i += 2 { + keyNode := node.Content[i] + if keyNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(keyNode) + } + key := keyNode.Value + valNode := node.Content[i+1] + switch key { + case "error": + if valNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(valNode) + } + var v string + if err := valNode.Decode(&v); err != nil { + return err + } + e.Error = v + case "len": + if valNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(valNode) + } + var v *int + if err := valNode.Decode(&v); err != nil { + return err + } + e.Len = v + case "unknown": + if valNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(valNode) + } + var v bool + if err := valNode.Decode(&v); err != nil { + return err + } + e.Unknown = v + case "notfound": + if valNode.Kind != yaml.ScalarNode { + return errors.ExpectedScalarAt(valNode) + } + var v bool + if err := valNode.Decode(&v); err != nil { + return err + } + e.NotFound = v + case "json": + if valNode.Kind != yaml.MappingNode { + return errors.ExpectedMapAt(valNode) + } + var v *gdtjson.Expect + if err := valNode.Decode(&v); err != nil { + return err + } + e.JSON = v + case "conditions": + if valNode.Kind != yaml.MappingNode { + return errors.ExpectedMapAt(valNode) + } + var v map[string]*ConditionMatch + if err := valNode.Decode(&v); err != nil { + return err + } + e.Conditions = v + case "matches": + if valNode.Kind == yaml.MappingNode { + var v map[string]interface{} + if err := valNode.Decode(&v); err != nil { + return err + } + e.Matches = v + } else if valNode.Kind == yaml.ScalarNode { + if valNode.Tag == "!!null" { + return ExpectedMapOrYAMLStringAt(valNode) + } + var v string + if err := valNode.Decode(&v); err != nil { + return err + } + if err := validateFileExists(v); err != nil { + return err + } + // inline YAML. check it can be unmarshaled into a + // map[string]interface{} + var m map[string]interface{} + if err := yaml.Unmarshal([]byte(v), &m); err != nil { + return MatchesInvalidUnmarshalError(err) + } + e.Matches = m + } else { + return ExpectedMapOrYAMLStringAt(valNode) + } + default: + return errors.UnknownFieldAt(key, keyNode) + } + } + return nil +} + // validateShortcuts ensures that the test author has specified only a single // shortcut (e.g. `kube.create`) and that if a shortcut is specified, any // long-form KubeSpec is not present. @@ -198,14 +302,6 @@ func validateKubeSpec(s *Spec) error { } } } - if s.Assert != nil { - exp := s.Assert - if exp.Matches != nil { - if err := validateMatches(exp.Matches); err != nil { - return err - } - } - } return nil } @@ -254,29 +350,3 @@ func validateResourceIdentifier(subject string) error { } return nil } - -// validateMatches checks what the test author placed in the `Kube.Matches` -// field to see if it contains one of: -// * file path (and checks existence of this file) -// * inline YAML (and checks that can be unmarshaled) -// * map[string]interface{} -func validateMatches(matches interface{}) error { - switch matches.(type) { - case string: - v := matches.(string) - if probablyFilePath(v) { - return validateFileExists(v) - } - // inline YAML. Let's quickly check it can be unmarshaled into a - // map[string]interface{} - var m map[string]interface{} - if err := yaml.Unmarshal([]byte(v), &m); err != nil { - return MatchesInvalidUnmarshalError(err) - } - case map[string]interface{}: - return nil - default: - return MatchesInvalid(matches) - } - return nil -} diff --git a/parse_test.go b/parse_test.go index b47b53e..284e962 100644 --- a/parse_test.go +++ b/parse_test.go @@ -22,7 +22,7 @@ func currentDir() string { return filepath.Dir(filename) } -func TestBadDefaults(t *testing.T) { +func TestFailureBadDefaults(t *testing.T) { assert := assert.New(t) require := require.New(t) @@ -138,7 +138,7 @@ func TestFailureCreateFileNotFound(t *testing.T) { require.Nil(s) } -func TestDeleteFileNotFound(t *testing.T) { +func TestFailureDeleteFileNotFound(t *testing.T) { require := require.New(t) assert := assert.New(t) @@ -177,6 +177,19 @@ func TestFailureBadMatchesInvalidYAML(t *testing.T) { require.Nil(s) } +func TestFailureBadMatchesEmpty(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + + fp := filepath.Join("testdata", "parse", "fail", "bad-matches-empty.yaml") + + s, err := gdt.From(fp) + require.NotNil(err) + assert.ErrorIs(err, gdtkube.ErrExpectedMapOrYAMLString) + assert.ErrorIs(err, errors.ErrParse) + require.Nil(s) +} + func TestFailureBadMatchesNotMapAny(t *testing.T) { assert := assert.New(t) require := require.New(t) diff --git a/testdata/parse/fail/bad-matches-empty.yaml b/testdata/parse/fail/bad-matches-empty.yaml new file mode 100644 index 0000000..cfa1a26 --- /dev/null +++ b/testdata/parse/fail/bad-matches-empty.yaml @@ -0,0 +1,7 @@ +name: bad-matches-empty +description: "matches is not a string or map[string]interface{}" +tests: + - kube: + get: pods/mypod + assert: + matches: diff --git a/testdata/parse/fail/unknown-context-in-defaults.yaml b/testdata/parse/fail/unknown-context-in-defaults.yaml deleted file mode 100644 index efb9866..0000000 --- a/testdata/parse/fail/unknown-context-in-defaults.yaml +++ /dev/null @@ -1,9 +0,0 @@ -name: unknown-context-in-defaults -description: unknown kubecontext specified in defaults -defaults: - kube: - context: unknownctx -tests: - - kube: - create: testdata/manifests/nginx-pod.yaml - diff --git a/testdata/parse/fail/unknown-context.yaml b/testdata/parse/fail/unknown-context.yaml deleted file mode 100644 index 70a474f..0000000 --- a/testdata/parse/fail/unknown-context.yaml +++ /dev/null @@ -1,6 +0,0 @@ -name: unknown-context -description: unknown kubecontext specified in spec -tests: - - kube: - create: testdata/manifests/nginx-pod.yaml - context: unknownctx