From 2f96943aadac53023548b382ae8053d3562be23d Mon Sep 17 00:00:00 2001 From: TheDiveO Date: Fri, 5 Nov 2021 15:47:49 +0100 Subject: [PATCH] add Error() assertions on the final error value of multi-return values (#480) --- internal/assertion.go | 76 +++++++++++++++++++++++--------- internal/assertion_test.go | 48 +++++++++++++++++++- internal/async_assertion.go | 4 +- internal/async_assertion_test.go | 6 +-- types/types.go | 2 + 5 files changed, 110 insertions(+), 26 deletions(-) diff --git a/internal/assertion.go b/internal/assertion.go index b7573330d..b3c26889a 100644 --- a/internal/assertion.go +++ b/internal/assertion.go @@ -8,17 +8,22 @@ import ( ) type Assertion struct { - actualInput interface{} + actuals []interface{} // actual value plus all extra values + actualIndex int // value to pass to the matcher + vet vetinari // the vet to call before calling Gomega matcher offset int - extra []interface{} g *Gomega } +// ...obligatory discworld reference, as "vetineer" doesn't sound ... quite right. +type vetinari func(assertion *Assertion, optionalDescription ...interface{}) bool + func NewAssertion(actualInput interface{}, g *Gomega, offset int, extra ...interface{}) *Assertion { return &Assertion{ - actualInput: actualInput, + actuals: append([]interface{}{actualInput}, extra...), + actualIndex: 0, + vet: (*Assertion).vetActuals, offset: offset, - extra: extra, g: g, } } @@ -28,29 +33,39 @@ func (assertion *Assertion) WithOffset(offset int) types.Assertion { return assertion } +func (assertion *Assertion) Error() types.Assertion { + return &Assertion{ + actuals: assertion.actuals, + actualIndex: len(assertion.actuals) - 1, + vet: (*Assertion).vetError, + offset: assertion.offset, + g: assertion.g, + } +} + func (assertion *Assertion) Should(matcher types.GomegaMatcher, optionalDescription ...interface{}) bool { assertion.g.THelper() - return assertion.vetExtras(optionalDescription...) && assertion.match(matcher, true, optionalDescription...) + return assertion.vet(assertion, optionalDescription...) && assertion.match(matcher, true, optionalDescription...) } func (assertion *Assertion) ShouldNot(matcher types.GomegaMatcher, optionalDescription ...interface{}) bool { assertion.g.THelper() - return assertion.vetExtras(optionalDescription...) && assertion.match(matcher, false, optionalDescription...) + return assertion.vet(assertion, optionalDescription...) && assertion.match(matcher, false, optionalDescription...) } func (assertion *Assertion) To(matcher types.GomegaMatcher, optionalDescription ...interface{}) bool { assertion.g.THelper() - return assertion.vetExtras(optionalDescription...) && assertion.match(matcher, true, optionalDescription...) + return assertion.vet(assertion, optionalDescription...) && assertion.match(matcher, true, optionalDescription...) } func (assertion *Assertion) ToNot(matcher types.GomegaMatcher, optionalDescription ...interface{}) bool { assertion.g.THelper() - return assertion.vetExtras(optionalDescription...) && assertion.match(matcher, false, optionalDescription...) + return assertion.vet(assertion, optionalDescription...) && assertion.match(matcher, false, optionalDescription...) } func (assertion *Assertion) NotTo(matcher types.GomegaMatcher, optionalDescription ...interface{}) bool { assertion.g.THelper() - return assertion.vetExtras(optionalDescription...) && assertion.match(matcher, false, optionalDescription...) + return assertion.vet(assertion, optionalDescription...) && assertion.match(matcher, false, optionalDescription...) } func (assertion *Assertion) buildDescription(optionalDescription ...interface{}) string { @@ -66,7 +81,8 @@ func (assertion *Assertion) buildDescription(optionalDescription ...interface{}) } func (assertion *Assertion) match(matcher types.GomegaMatcher, desiredMatch bool, optionalDescription ...interface{}) bool { - matches, err := matcher.Match(assertion.actualInput) + actualInput := assertion.actuals[assertion.actualIndex] + matches, err := matcher.Match(actualInput) assertion.g.THelper() if err != nil { description := assertion.buildDescription(optionalDescription...) @@ -76,9 +92,9 @@ func (assertion *Assertion) match(matcher types.GomegaMatcher, desiredMatch bool if matches != desiredMatch { var message string if desiredMatch { - message = matcher.FailureMessage(assertion.actualInput) + message = matcher.FailureMessage(actualInput) } else { - message = matcher.NegatedFailureMessage(assertion.actualInput) + message = matcher.NegatedFailureMessage(actualInput) } description := assertion.buildDescription(optionalDescription...) assertion.g.Fail(description+message, 2+assertion.offset) @@ -88,8 +104,11 @@ func (assertion *Assertion) match(matcher types.GomegaMatcher, desiredMatch bool return true } -func (assertion *Assertion) vetExtras(optionalDescription ...interface{}) bool { - success, message := vetExtras(assertion.extra) +// vetActuals vets the actual values, with the (optional) exception of a +// specific value, such as the first value in case non-error assertions, or the +// last value in case of Error()-based assertions. +func (assertion *Assertion) vetActuals(optionalDescription ...interface{}) bool { + success, message := vetActuals(assertion.actuals, assertion.actualIndex) if success { return true } @@ -100,12 +119,29 @@ func (assertion *Assertion) vetExtras(optionalDescription ...interface{}) bool { return false } -func vetExtras(extras []interface{}) (bool, string) { - for i, extra := range extras { - if extra != nil { - zeroValue := reflect.Zero(reflect.TypeOf(extra)).Interface() - if !reflect.DeepEqual(zeroValue, extra) { - message := fmt.Sprintf("Unexpected non-nil/non-zero extra argument at index %d:\n\t<%T>: %#v", i+1, extra, extra) +// vetError vets the actual values, except for the final error value, in case +// the final error value is non-zero. Otherwise, it doesn't vet the actual +// values, as these are allowed to take on any values unless there is a non-zero +// error value. +func (assertion *Assertion) vetError(optionalDescription ...interface{}) bool { + if err := assertion.actuals[assertion.actualIndex]; err != nil { + // Go error result idiom: all other actual values must be zero values. + return assertion.vetActuals(optionalDescription...) + } + return true +} + +// vetActuals vets a slice of actual values, optionally skipping a particular +// value slice element, such as the first or last value slice element. +func vetActuals(actuals []interface{}, skipIndex int) (bool, string) { + for i, actual := range actuals { + if i == skipIndex { + continue + } + if actual != nil { + zeroValue := reflect.Zero(reflect.TypeOf(actual)).Interface() + if !reflect.DeepEqual(zeroValue, actual) { + message := fmt.Sprintf("Unexpected non-nil/non-zero argument at index %d:\n\t<%T>: %#v", i, actual, actual) return false, message } } diff --git a/internal/assertion_test.go b/internal/assertion_test.go index a02aab0d4..444c50cb3 100644 --- a/internal/assertion_test.go +++ b/internal/assertion_test.go @@ -1,6 +1,8 @@ package internal_test import ( + "errors" + . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" @@ -141,7 +143,51 @@ var _ = Describe("Making Synchronous Assertions", func() { Entry( "when the matcher matches but a non-zero-valued extra parameter is included, it fails", MATCH, Extras(1, "bam", struct{ Foo string }{Foo: "foo"}, nil), OptionalDescription(), - SHOULD_MATCH, "Unexpected non-nil/non-zero extra argument at index 1:\n\t: 1", IT_FAILS, + SHOULD_MATCH, "Unexpected non-nil/non-zero argument at index 1:\n\t: 1", IT_FAILS, ), ) + + var SHOULD_OCCUR = true + var SHOULD_NOT_OCCUR = false + + DescribeTable("error expectations", + func(a, b int, e error, isPositiveAssertion bool, expectedFailureMessage string, expectedReturnValue bool) { + abe := func(a, b int, e error) (int, int, error) { + return a, b, e + } + ig := NewInstrumentedGomega() + var returnValue bool + if isPositiveAssertion { + returnValue = ig.G.Expect(abe(a, b, e)).Error().To(HaveOccurred()) + } else { + returnValue = ig.G.Expect(abe(a, b, e)).Error().NotTo(HaveOccurred()) + } + Expect(returnValue).To(Equal(expectedReturnValue)) + Expect(ig.FailureMessage).To(Equal(expectedFailureMessage)) + if expectedFailureMessage != "" { + Expect(ig.FailureSkip).To(Equal([]int{2})) + } + }, + Entry( + "when non-zero results without error", + 1, 2, nil, + SHOULD_NOT_OCCUR, "", IT_PASSES, + ), + Entry( + "when non-zero results with error", + 1, 2, errors.New("D'oh!"), + SHOULD_NOT_OCCUR, "Unexpected non-nil/non-zero argument at index 0:\n\t: 1", IT_FAILS, + ), + Entry( + "when non-zero results without error", + 0, 0, errors.New("D'oh!"), + SHOULD_OCCUR, "", IT_PASSES, + ), + Entry( + "when non-zero results with error", + 1, 2, errors.New("D'oh!"), + SHOULD_OCCUR, "Unexpected non-nil/non-zero argument at index 0:\n\t: 1", IT_FAILS, + ), + ) + }) diff --git a/internal/async_assertion.go b/internal/async_assertion.go index 3e52992cf..99f4ebcfe 100644 --- a/internal/async_assertion.go +++ b/internal/async_assertion.go @@ -133,11 +133,11 @@ func (assertion *AsyncAssertion) pollActual() (interface{}, error) { if err != nil { return nil, err } - extras := []interface{}{} + extras := []interface{}{nil} for _, value := range values[1:] { extras = append(extras, value.Interface()) } - success, message := vetExtras(extras) + success, message := vetActuals(extras, 0) if !success { return nil, errors.New(message) } diff --git a/internal/async_assertion_test.go b/internal/async_assertion_test.go index 6e0284dc6..5d16fcb71 100644 --- a/internal/async_assertion_test.go +++ b/internal/async_assertion_test.go @@ -331,7 +331,7 @@ var _ = Describe("Asynchronous Assertions", func() { ig.G.Eventually(func() (int, string, Foo, error) { return 1, "", Foo{Bar: "hi"}, nil }).WithTimeout(30 * time.Millisecond).WithPolling(10 * time.Millisecond).Should(BeNumerically("<", 100)) - Ω(ig.FailureMessage).Should(ContainSubstring("Error: Unexpected non-nil/non-zero extra argument at index 2:")) + Ω(ig.FailureMessage).Should(ContainSubstring("Error: Unexpected non-nil/non-zero argument at index 2:")) Ω(ig.FailureMessage).Should(ContainSubstring(`Foo{Bar:"hi"}`)) }) @@ -377,7 +377,7 @@ var _ = Describe("Asynchronous Assertions", func() { } return counter, s, f, err }).WithTimeout(50 * time.Millisecond).WithPolling(10 * time.Millisecond).Should(BeNumerically("<", 100)) - Ω(ig.FailureMessage).Should(ContainSubstring("Error: Unexpected non-nil/non-zero extra argument at index 2:")) + Ω(ig.FailureMessage).Should(ContainSubstring("Error: Unexpected non-nil/non-zero argument at index 2:")) Ω(ig.FailureMessage).Should(ContainSubstring(`Foo{Bar:"welp"}`)) Ω(counter).Should(Equal(3)) }) @@ -404,7 +404,7 @@ var _ = Describe("Asynchronous Assertions", func() { } return counter, s, f, err }).WithTimeout(50 * time.Millisecond).WithPolling(10 * time.Millisecond).ShouldNot(BeNumerically(">", 100)) - Ω(ig.FailureMessage).Should(ContainSubstring("Error: Unexpected non-nil/non-zero extra argument at index 1:")) + Ω(ig.FailureMessage).Should(ContainSubstring("Error: Unexpected non-nil/non-zero argument at index 1:")) Ω(ig.FailureMessage).Should(ContainSubstring(`: "welp"`)) Ω(counter).Should(Equal(3)) }) diff --git a/types/types.go b/types/types.go index a0ae6cbc9..c315ef065 100644 --- a/types/types.go +++ b/types/types.go @@ -82,4 +82,6 @@ type Assertion interface { NotTo(matcher GomegaMatcher, optionalDescription ...interface{}) bool WithOffset(offset int) Assertion + + Error() Assertion }