extract failure messages into separate methods

Previously formatting early could lead to race conditions as it formatted the
object under test.

(This also makes the interface a bit clearer.)

Signed-off-by: Ted Young <tyoung@pivotallabs.com>

README.md

@@ -12,4 +12,4 @@ Learn more about Ginkgo [here](http://onsi.github.io/ginkgo/)
 
 ## License
 
-Gomega is MIT-Licensed
+Gomega is MIT-Licensed

actual.go

@@ -51,13 +51,19 @@ func (actual *actual) buildDescription(optionalDescription ...interface{}) strin
 }
 
 func (actual *actual) match(matcher OmegaMatcher, desiredMatch bool, optionalDescription ...interface{}) bool {
-	matches, message, err := matcher.Match(actual.actualInput)
+	matches, err := matcher.Match(actual.actualInput)
 	description := actual.buildDescription(optionalDescription...)
 	if err != nil {
 		actual.fail(description+err.Error(), 2+actual.offset)
 		return false
 	}
 	if matches != desiredMatch {
+		var message string
+		if desiredMatch {
+			message = matcher.FailureMessage(actual)
+		} else {
+			message = matcher.NegatedFailureMessage(actual)
+		}
 		actual.fail(description+message, 2+actual.offset)
 		return false
 	}

async_actual.go

@@ -87,18 +87,24 @@ func (actual *asyncActual) match(matcher OmegaMatcher, desiredMatch bool, option
 	description := actual.buildDescription(optionalDescription...)
 
 	var matches bool
-	var message string
 	var err error
 	value, err := actual.pollActual()
 	if err == nil {
-		matches, message, err = matcher.Match(value)
+		matches, err = matcher.Match(value)
 	}
 
 	fail := func(preamble string) {
 		errMsg := ""
 		if err != nil {
 			errMsg = "Error: " + err.Error()
 		}
+
+		var message string
+		if desiredMatch {
+			message = matcher.FailureMessage(actual)
+		} else {
+			message = matcher.NegatedFailureMessage(actual)
+		}
 		actual.fail(fmt.Sprintf("%s after %.3fs.\n%s%s%s", preamble, time.Since(timer).Seconds(), description, message, errMsg), 3+actual.offset)
 	}
 
@@ -112,7 +118,7 @@ func (actual *asyncActual) match(matcher OmegaMatcher, desiredMatch bool, option
 			case <-time.After(actual.pollingInterval):
 				value, err = actual.pollActual()
 				if err == nil {
-					matches, message, err = matcher.Match(value)
+					matches, err = matcher.Match(value)
 				}
 			case <-timeout:
 				fail("Timed out")
@@ -130,7 +136,7 @@ func (actual *asyncActual) match(matcher OmegaMatcher, desiredMatch bool, option
 			case <-time.After(actual.pollingInterval):
 				value, err = actual.pollActual()
 				if err == nil {
-					matches, message, err = matcher.Match(value)
+					matches, err = matcher.Match(value)
 				}
 			case <-timeout:
 				return true

gomega.go

@@ -218,5 +218,7 @@ type Actual interface {
 //
 //For details on writing custom matchers, check out: http://onsi.github.io/gomega/#adding_your_own_matchers
 type OmegaMatcher interface {
-	Match(actual interface{}) (success bool, message string, err error)
+	Match(actual interface{}) (success bool, err error)
+	FailureMessage(actual interface{}) (message string)
+	NegatedFailureMessage(actual interface{}) (message string)
 }

gomega_suite_test.go

@@ -22,8 +22,16 @@ type fakeMatcher struct {
 	errToReturn     error
 }
 
-func (matcher *fakeMatcher) Match(actual interface{}) (bool, string, error) {
+func (matcher *fakeMatcher) Match(actual interface{}) (bool, error) {
 	matcher.receivedActual = actual
 
-	return matcher.matchesToReturn, matcher.messageToReturn, matcher.errToReturn
+	return matcher.matchesToReturn, matcher.errToReturn
+}
+
+func (matcher *fakeMatcher) FailureMessage(actual interface{}) string {
+	return matcher.messageToReturn
+}
+
+func (matcher *fakeMatcher) NegatedFailureMessage(actual interface{}) string {
+	return matcher.messageToReturn
 }

matchers/assignable_to_type_of_matcher.go

@@ -2,25 +2,29 @@ package matchers
 
 import (
 	"fmt"
-	"reflect"
 	"github.com/onsi/gomega/format"
+	"reflect"
 )
 
 type AssignableToTypeOfMatcher struct {
 	Expected interface{}
 }
 
-func (matcher *AssignableToTypeOfMatcher) Match(actual interface{}) (success bool, message string, err error) {
+func (matcher *AssignableToTypeOfMatcher) Match(actual interface{}) (success bool, err error) {
 	if actual == nil || matcher.Expected == nil {
-		return false, "", fmt.Errorf("Refusing to compare <nil> to <nil>.")
+		return false, fmt.Errorf("Refusing to compare <nil> to <nil>.")
 	}
 
 	actualType := reflect.TypeOf(actual)
 	expectedType := reflect.TypeOf(matcher.Expected)
 
-	if actualType.AssignableTo(expectedType) {
-		return true, format.Message(actual, fmt.Sprintf("not to be assignable to the type: %T", matcher.Expected)), nil
-	} else {
-		return false, format.Message(actual, fmt.Sprintf("to be assignable to the type: %T", matcher.Expected)), nil
-	}
+	return actualType.AssignableTo(expectedType), nil
+}
+
+func (matcher *AssignableToTypeOfMatcher) FailureMessage(actual interface{}) string {
+	return format.Message(actual, fmt.Sprintf("not to be assignable to the type: %T", matcher.Expected))
+}
+
+func (matcher *AssignableToTypeOfMatcher) NegatedFailureMessage(actual interface{}) string {
+	return format.Message(actual, fmt.Sprintf("not to be assignable to the type: %T", matcher.Expected))
 }

matchers/assignable_to_type_of_matcher_test.go

@@ -22,8 +22,7 @@ var _ = Describe("AssignableToTypeOf", func() {
 
 	Context("When asserting nil values", func() {
 		It("should error", func() {
-			success, _, err := (&AssignableToTypeOfMatcher{Expected: nil}).Match(nil)
-
+			success, err := (&AssignableToTypeOfMatcher{Expected: nil}).Match(nil)
 			Ω(success).Should(BeFalse())
 			Ω(err).Should(HaveOccurred())
 		})

matchers/be_closed_matcher.go

@@ -9,34 +9,37 @@ import (
 type BeClosedMatcher struct {
 }
 
-func (matcher *BeClosedMatcher) Match(actual interface{}) (success bool, message string, err error) {
+func (matcher *BeClosedMatcher) Match(actual interface{}) (success bool, err error) {
 	if !isChan(actual) {
-		return false, "", fmt.Errorf("BeClosed matcher expects a channel.  Got:\n%s", format.Object(actual, 1))
+		return false, fmt.Errorf("BeClosed matcher expects a channel.  Got:\n%s", format.Object(actual, 1))
 	}
 
 	channelType := reflect.TypeOf(actual)
 	channelValue := reflect.ValueOf(actual)
 
-	var closed bool
-
 	if channelType.ChanDir() == reflect.SendDir {
-		return false, "", fmt.Errorf("BeClosed matcher cannot determine if a send-only channel is closed or open.  Got:\n%s", format.Object(actual, 1))
-	} else {
-		winnerIndex, _, open := reflect.Select([]reflect.SelectCase{
-			reflect.SelectCase{Dir: reflect.SelectRecv, Chan: channelValue},
-			reflect.SelectCase{Dir: reflect.SelectDefault},
-		})
-
-		if winnerIndex == 0 {
-			closed = !open
-		} else if winnerIndex == 1 {
-			closed = false
-		}
+		return false, fmt.Errorf("BeClosed matcher cannot determine if a send-only channel is closed or open.  Got:\n%s", format.Object(actual, 1))
 	}
 
-	if closed {
-		return true, format.Message(actual, "to be open"), nil
-	} else {
-		return false, format.Message(actual, "to be closed"), nil
+	winnerIndex, _, open := reflect.Select([]reflect.SelectCase{
+		reflect.SelectCase{Dir: reflect.SelectRecv, Chan: channelValue},
+		reflect.SelectCase{Dir: reflect.SelectDefault},
+	})
+
+	var closed bool
+	if winnerIndex == 0 {
+		closed = !open
+	} else if winnerIndex == 1 {
+		closed = false
 	}
+
+	return closed, nil
+}
+
+func (matcher *BeClosedMatcher) FailureMessage(actual interface{}) (message string) {
+	return format.Message(actual, "to be closed")
+}
+
+func (matcher *BeClosedMatcher) NegatedFailureMessage(actual interface{}) (message string) {
+	return format.Message(actual, "to be open")
 }

matchers/be_closed_matcher_test.go

@@ -33,7 +33,7 @@ var _ = Describe("BeClosedMatcher", func() {
 			var openWriterChannel chan<- bool
 			openWriterChannel = openChannel
 
-			success, _, err := (&BeClosedMatcher{}).Match(openWriterChannel)
+			success, err := (&BeClosedMatcher{}).Match(openWriterChannel)
 			Ω(success).Should(BeFalse())
 			Ω(err).Should(HaveOccurred())
 
@@ -43,7 +43,7 @@ var _ = Describe("BeClosedMatcher", func() {
 			var closedWriterChannel chan<- bool
 			closedWriterChannel = closedChannel
 
-			success, _, err = (&BeClosedMatcher{}).Match(closedWriterChannel)
+			success, err = (&BeClosedMatcher{}).Match(closedWriterChannel)
 			Ω(success).Should(BeFalse())
 			Ω(err).Should(HaveOccurred())
 
@@ -54,15 +54,15 @@ var _ = Describe("BeClosedMatcher", func() {
 		It("should error", func() {
 			var nilChannel chan bool
 
-			success, _, err := (&BeClosedMatcher{}).Match(nilChannel)
+			success, err := (&BeClosedMatcher{}).Match(nilChannel)
 			Ω(success).Should(BeFalse())
 			Ω(err).Should(HaveOccurred())
 
-			success, _, err = (&BeClosedMatcher{}).Match(nil)
+			success, err = (&BeClosedMatcher{}).Match(nil)
 			Ω(success).Should(BeFalse())
 			Ω(err).Should(HaveOccurred())
 
-			success, _, err = (&BeClosedMatcher{}).Match(7)
+			success, err = (&BeClosedMatcher{}).Match(7)
 			Ω(success).Should(BeFalse())
 			Ω(err).Should(HaveOccurred())
 		})

matchers/be_empty_matcher.go

@@ -8,15 +8,19 @@ import (
 type BeEmptyMatcher struct {
 }
 
-func (matcher *BeEmptyMatcher) Match(actual interface{}) (success bool, message string, err error) {
+func (matcher *BeEmptyMatcher) Match(actual interface{}) (success bool, err error) {
 	length, ok := lengthOf(actual)
-	if ok {
-		if length == 0 {
-			return true, format.Message(actual, "not to be empty"), nil
-		} else {
-			return false, format.Message(actual, "to be empty"), nil
-		}
-	} else {
-		return false, "", fmt.Errorf("BeEmpty matcher expects a string/array/map/channel/slice.  Got:\n%s", format.Object(actual, 1))
+	if !ok {
+		return false, fmt.Errorf("BeEmpty matcher expects a string/array/map/channel/slice.  Got:\n%s", format.Object(actual, 1))
 	}
+
+	return length == 0, nil
+}
+
+func (matcher *BeEmptyMatcher) FailureMessage(actual interface{}) (message string) {
+	return format.Message(actual, "to be empty")
+}
+
+func (matcher *BeEmptyMatcher) NegatedFailureMessage(actual interface{}) (message string) {
+	return format.Message(actual, "not to be empty")
 }

matchers/be_empty_matcher_test.go

@@ -40,11 +40,11 @@ var _ = Describe("BeEmpty", func() {
 
 	Context("when passed an unsupported type", func() {
 		It("should error", func() {
-			success, _, err := (&BeEmptyMatcher{}).Match(0)
+			success, err := (&BeEmptyMatcher{}).Match(0)
 			Ω(success).Should(BeFalse())
 			Ω(err).Should(HaveOccurred())
 
-			success, _, err = (&BeEmptyMatcher{}).Match(nil)
+			success, err = (&BeEmptyMatcher{}).Match(nil)
 			Ω(success).Should(BeFalse())
 			Ω(err).Should(HaveOccurred())
 		})

matchers/be_equivalent_to_matcher.go

@@ -10,9 +10,9 @@ type BeEquivalentToMatcher struct {
 	Expected interface{}
 }
 
-func (matcher *BeEquivalentToMatcher) Match(actual interface{}) (success bool, message string, err error) {
+func (matcher *BeEquivalentToMatcher) Match(actual interface{}) (success bool, err error) {
 	if actual == nil && matcher.Expected == nil {
-		return false, "", fmt.Errorf("Both actual and expected must not be nil.")
+		return false, fmt.Errorf("Both actual and expected must not be nil.")
 	}
 
 	convertedActual := actual
@@ -21,9 +21,13 @@ func (matcher *BeEquivalentToMatcher) Match(actual interface{}) (success bool, m
 		convertedActual = reflect.ValueOf(actual).Convert(reflect.TypeOf(matcher.Expected)).Interface()
 	}
 
-	if reflect.DeepEqual(convertedActual, matcher.Expected) {
-		return true, format.Message(actual, "not to be equivalent to", matcher.Expected), nil
-	} else {
-		return false, format.Message(actual, "to be equivalent to", matcher.Expected), nil
-	}
+	return reflect.DeepEqual(convertedActual, matcher.Expected), nil
+}
+
+func (matcher *BeEquivalentToMatcher) FailureMessage(actual interface{}) (message string) {
+	return format.Message(actual, "to be equivalent to", matcher.Expected)
+}
+
+func (matcher *BeEquivalentToMatcher) NegatedFailureMessage(actual interface{}) (message string) {
+	return format.Message(actual, "not to be equivalent to", matcher.Expected)
 }

matchers/be_equivalent_to_matcher_test.go

@@ -9,7 +9,7 @@ import (
 var _ = Describe("BeEquivalentTo", func() {
 	Context("when asserting that nil is equivalent to nil", func() {
 		It("should error", func() {
-			success, _, err := (&BeEquivalentToMatcher{Expected: nil}).Match(nil)
+			success, err := (&BeEquivalentToMatcher{Expected: nil}).Match(nil)
 
 			Ω(success).Should(BeFalse())
 			Ω(err).Should(HaveOccurred())

matchers/be_false_matcher.go

@@ -8,13 +8,18 @@ import (
 type BeFalseMatcher struct {
 }
 
-func (matcher *BeFalseMatcher) Match(actual interface{}) (success bool, message string, err error) {
+func (matcher *BeFalseMatcher) Match(actual interface{}) (success bool, err error) {
 	if !isBool(actual) {
-		return false, "", fmt.Errorf("Expected a boolean.  Got:\n%s", format.Object(actual, 1))
-	}
-	if actual == false {
-		return true, format.Message(actual, "not to be false"), nil
-	} else {
-		return false, format.Message(actual, "to be false"), nil
+		return false, fmt.Errorf("Expected a boolean.  Got:\n%s", format.Object(actual, 1))
 	}
+
+	return actual == false, nil
+}
+
+func (matcher *BeFalseMatcher) FailureMessage(actual interface{}) (message string) {
+	return format.Message(actual, "to be false")
+}
+
+func (matcher *BeFalseMatcher) NegatedFailureMessage(actual interface{}) (message string) {
+	return format.Message(actual, "not to be false")
 }

matchers/be_false_matcher_test.go

@@ -13,7 +13,7 @@ var _ = Describe("BeFalse", func() {
 	})
 
 	It("should only support booleans", func() {
-		success, _, err := (&BeFalseMatcher{}).Match("foo")
+		success, err := (&BeFalseMatcher{}).Match("foo")
 		Ω(success).Should(BeFalse())
 		Ω(err).Should(HaveOccurred())
 	})

matchers/be_nil_matcher.go

@@ -5,10 +5,14 @@ import "github.com/onsi/gomega/format"
 type BeNilMatcher struct {
 }
 
-func (matcher *BeNilMatcher) Match(actual interface{}) (success bool, message string, err error) {
-	if isNil(actual) {
-		return true, format.Message(actual, "not to be nil"), nil
-	} else {
-		return false, format.Message(actual, "to be nil"), nil
-	}
+func (matcher *BeNilMatcher) Match(actual interface{}) (success bool, err error) {
+	return isNil(actual), nil
+}
+
+func (matcher *BeNilMatcher) FailureMessage(actual interface{}) (message string) {
+	return format.Message(actual, "to be nil")
+}
+
+func (matcher *BeNilMatcher) NegatedFailureMessage(actual interface{}) (message string) {
+	return format.Message(actual, "not to be nil")
 }