From 381bed1fd8da40c28d50eca6edd4c5844439e827 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Tue, 19 Feb 2019 17:56:12 +0000 Subject: [PATCH] Print unit test failures in action matchers as part of failure message Signed-off-by: James Munnelly --- pkg/controller/test/actions.go | 25 +++++++++--------------- pkg/controller/test/context_builder.go | 27 +++++++++++++++++++++----- pkg/issuer/acme/issue_test.go | 15 ++++++++++---- 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/pkg/controller/test/actions.go b/pkg/controller/test/actions.go index ac86f08c7..a977dbd8d 100644 --- a/pkg/controller/test/actions.go +++ b/pkg/controller/test/actions.go @@ -17,19 +17,18 @@ limitations under the License. package test import ( - "encoding/json" - "log" + "fmt" "reflect" "github.com/kr/pretty" coretesting "k8s.io/client-go/testing" ) -type ActionMatchFn func(coretesting.Action, coretesting.Action) bool +type ActionMatchFn func(coretesting.Action, coretesting.Action) error type Action interface { Action() coretesting.Action - Matches(coretesting.Action) bool + Matches(coretesting.Action) error } type customMatchAction struct { @@ -50,7 +49,7 @@ func (a *customMatchAction) Action() coretesting.Action { return a.action } -func (a *customMatchAction) Matches(act coretesting.Action) bool { +func (a *customMatchAction) Matches(act coretesting.Action) error { return a.matchFn(a.action, act) } @@ -70,26 +69,20 @@ func (a *action) Action() coretesting.Action { return a.action } -func (a *action) Matches(act coretesting.Action) bool { +func (a *action) Matches(act coretesting.Action) error { matches := reflect.DeepEqual(a.action, act) if matches == true { - return true + return nil } objAct, ok := act.(coretesting.CreateAction) if !ok { - return false + return nil } objExp, ok := a.action.(coretesting.CreateAction) if !ok { - return false + return nil } - bExp, _ := json.MarshalIndent(objExp, "", "\t") - bAct, _ := json.MarshalIndent(objAct, "", "\t") - log.Printf("Expected: %s", string(bExp)) - log.Printf("Actual: %s", string(bAct)) - log.Printf("Unexpected difference between actions: %s", pretty.Diff(objExp.GetObject(), objAct.GetObject())) - - return false + return fmt.Errorf("unexpected difference between actions: %s", pretty.Diff(objExp.GetObject(), objAct.GetObject())) } diff --git a/pkg/controller/test/context_builder.go b/pkg/controller/test/context_builder.go index edec3c861..51fd36455 100644 --- a/pkg/controller/test/context_builder.go +++ b/pkg/controller/test/context_builder.go @@ -159,6 +159,7 @@ func (b *Builder) AllActionsExecuted() error { firedActions = append(firedActions, b.FakeKubeClient().Actions()...) var unexpectedActions []coretesting.Action + var errs []error missingActions := make([]Action, len(b.ExpectedActions)) copy(missingActions, b.ExpectedActions) for _, a := range firedActions { @@ -167,18 +168,34 @@ func (b *Builder) AllActionsExecuted() error { continue } found := false + var err error for i, expA := range missingActions { - if expA.Matches(a) { - missingActions = append(missingActions[:i], missingActions[i+1:]...) - found = true - break + if expA.Action().GetNamespace() != a.GetNamespace() || + expA.Action().GetResource() != a.GetResource() || + expA.Action().GetSubresource() != a.GetSubresource() || + expA.Action().GetVerb() != a.GetVerb() { + continue } + + err = expA.Matches(a) + // if this action doesn't match, we record the error and continue + // as there may be multiple action matchers for the same resource + if err != nil { + continue + } + + missingActions = append(missingActions[:i], missingActions[i+1:]...) + found = true + break } if !found { unexpectedActions = append(unexpectedActions, a) + + if err != nil { + errs = append(errs, err) + } } } - var errs []error for _, a := range missingActions { errs = append(errs, fmt.Errorf("missing action: %v", actionToString(a.Action()))) } diff --git a/pkg/issuer/acme/issue_test.go b/pkg/issuer/acme/issue_test.go index 7f9d7fc47..6210b6045 100644 --- a/pkg/issuer/acme/issue_test.go +++ b/pkg/issuer/acme/issue_test.go @@ -24,6 +24,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" + "fmt" "math/big" "reflect" "testing" @@ -175,12 +176,15 @@ func TestIssueHappyPath(t *testing.T) { KubeObjects: []runtime.Object{testCertPrivateKeySecret}, ExpectedActions: []testpkg.Action{ testpkg.NewCustomMatch(coretesting.NewCreateAction(v1alpha1.SchemeGroupVersion.WithResource("orders"), testCertEmptyOrder.Namespace, testCertEmptyOrder), - func(exp, actual coretesting.Action) bool { + func(exp, actual coretesting.Action) error { expOrder := exp.(coretesting.CreateAction).GetObject().(*v1alpha1.Order) actOrder := actual.(coretesting.CreateAction).GetObject().(*v1alpha1.Order) expOrderCopy := expOrder.DeepCopy() expOrderCopy.Spec.CSR = actOrder.Spec.CSR - return reflect.DeepEqual(expOrderCopy, actOrder) + if !reflect.DeepEqual(expOrderCopy, actOrder) { + return fmt.Errorf("unexpected difference: %s", pretty.Diff(expOrderCopy, actOrder)) + } + return nil }), }, }, @@ -395,12 +399,15 @@ func TestIssueRetryCases(t *testing.T) { coretesting.NewDeleteAction(v1alpha1.SchemeGroupVersion.WithResource("orders"), invalidTestOrder.Namespace, invalidTestOrder.Name), ), testpkg.NewCustomMatch(coretesting.NewCreateAction(v1alpha1.SchemeGroupVersion.WithResource("orders"), testOrder.Namespace, testOrder), - func(exp, actual coretesting.Action) bool { + func(exp, actual coretesting.Action) error { expOrder := exp.(coretesting.CreateAction).GetObject().(*v1alpha1.Order) actOrder := actual.(coretesting.CreateAction).GetObject().(*v1alpha1.Order) expOrderCopy := expOrder.DeepCopy() expOrderCopy.Spec.CSR = actOrder.Spec.CSR - return reflect.DeepEqual(expOrderCopy, actOrder) + if !reflect.DeepEqual(expOrderCopy, actOrder) { + return fmt.Errorf("unexpected difference: %s", pretty.Diff(expOrderCopy, actOrder)) + } + return nil }), }, },