Better output when using with go XUnit-style tests, fixes #255 (#297)

* Improve stack traces in go test tests

* improve XUnit output for versions of Go that do not implement T.Helper

* [CHANGELOG.md] - put changes at the top

Author: blgm

CHANGELOG.md

@@ -1,3 +1,9 @@
+## HEAD
+
+Improvements:
+
+- XUnit style golang tests have much more useful stack trace in Golang 1.9 thanks to t.Helper.
+
 ## 1.4.1
 
 ### Fixes:

gomega_dsl.go

@@ -32,7 +32,7 @@ Alternatively, you may have forgotten to register a fail handler with RegisterFa
 Depending on your vendoring solution you may be inadvertently importing gomega and subpackages (e.g. ghhtp, gexec,...) from different locations.
 `
 
-var globalFailHandler types.GomegaFailHandler
+var globalFailWrapper *types.GomegaFailWrapper
 
 var defaultEventuallyTimeout = time.Second
 var defaultEventuallyPollingInterval = 10 * time.Millisecond
@@ -42,7 +42,14 @@ var defaultConsistentlyPollingInterval = 10 * time.Millisecond
 //RegisterFailHandler connects Ginkgo to Gomega.  When a matcher fails
 //the fail handler passed into RegisterFailHandler is called.
 func RegisterFailHandler(handler types.GomegaFailHandler) {
-	globalFailHandler = handler
+	if handler == nil {
+		globalFailWrapper = nil
+		return
+	}
+	globalFailWrapper = &types.GomegaFailWrapper{
+		Fail:        handler,
+		TWithHelper: testingtsupport.EmptyTWithHelper{},
+	}
 }
 
 //RegisterTestingT connects Gomega to Golang's XUnit style
@@ -67,7 +74,7 @@ func RegisterFailHandler(handler types.GomegaFailHandler) {
 //
 // (As an aside: Ginkgo gets around this limitation by running parallel tests in different *processes*).
 func RegisterTestingT(t types.GomegaTestingT) {
-	RegisterFailHandler(testingtsupport.BuildTestingTGomegaFailHandler(t))
+	RegisterFailHandler(testingtsupport.BuildTestingTGomegaFailWrapper(t).Fail)
 }
 
 //InterceptGomegaHandlers runs a given callback and returns an array of
@@ -80,7 +87,7 @@ func RegisterTestingT(t types.GomegaTestingT) {
 //This is most useful when testing custom matchers, but can also be used to check
 //on a value using a Gomega assertion without causing a test failure.
 func InterceptGomegaFailures(f func()) []string {
-	originalHandler := globalFailHandler
+	originalHandler := globalFailWrapper.Fail
 	failures := []string{}
 	RegisterFailHandler(func(message string, callerSkip ...int) {
 		failures = append(failures, message)
@@ -142,10 +149,10 @@ func Expect(actual interface{}, extra ...interface{}) GomegaAssertion {
 //error message to refer to the calling line in the test (as opposed to the line in the helper function)
 //set the first argument of `ExpectWithOffset` appropriately.
 func ExpectWithOffset(offset int, actual interface{}, extra ...interface{}) GomegaAssertion {
-	if globalFailHandler == nil {
+	if globalFailWrapper == nil {
 		panic(nilFailHandlerPanic)
 	}
-	return assertion.New(actual, globalFailHandler, offset, extra...)
+	return assertion.New(actual, globalFailWrapper, offset, extra...)
 }
 
 //Eventually wraps an actual value allowing assertions to be made on it.
@@ -192,7 +199,7 @@ func Eventually(actual interface{}, intervals ...interface{}) GomegaAsyncAsserti
 //initial argument to indicate an offset in the call stack.  This is useful when building helper
 //functions that contain matchers.  To learn more, read about `ExpectWithOffset`.
 func EventuallyWithOffset(offset int, actual interface{}, intervals ...interface{}) GomegaAsyncAssertion {
-	if globalFailHandler == nil {
+	if globalFailWrapper == nil {
 		panic(nilFailHandlerPanic)
 	}
 	timeoutInterval := defaultEventuallyTimeout
@@ -203,7 +210,7 @@ func EventuallyWithOffset(offset int, actual interface{}, intervals ...interface
 	if len(intervals) > 1 {
 		pollingInterval = toDuration(intervals[1])
 	}
-	return asyncassertion.New(asyncassertion.AsyncAssertionTypeEventually, actual, globalFailHandler, timeoutInterval, pollingInterval, offset)
+	return asyncassertion.New(asyncassertion.AsyncAssertionTypeEventually, actual, globalFailWrapper, timeoutInterval, pollingInterval, offset)
 }
 
 //Consistently wraps an actual value allowing assertions to be made on it.
@@ -237,7 +244,7 @@ func Consistently(actual interface{}, intervals ...interface{}) GomegaAsyncAsser
 //initial argument to indicate an offset in the call stack.  This is useful when building helper
 //functions that contain matchers.  To learn more, read about `ExpectWithOffset`.
 func ConsistentlyWithOffset(offset int, actual interface{}, intervals ...interface{}) GomegaAsyncAssertion {
-	if globalFailHandler == nil {
+	if globalFailWrapper == nil {
 		panic(nilFailHandlerPanic)
 	}
 	timeoutInterval := defaultConsistentlyDuration
@@ -248,7 +255,7 @@ func ConsistentlyWithOffset(offset int, actual interface{}, intervals ...interfa
 	if len(intervals) > 1 {
 		pollingInterval = toDuration(intervals[1])
 	}
-	return asyncassertion.New(asyncassertion.AsyncAssertionTypeConsistently, actual, globalFailHandler, timeoutInterval, pollingInterval, offset)
+	return asyncassertion.New(asyncassertion.AsyncAssertionTypeConsistently, actual, globalFailWrapper, timeoutInterval, pollingInterval, offset)
 }
 
 //Set the default timeout duration for Eventually.  Eventually will repeatedly poll your condition until it succeeds, or until this timeout elapses.
@@ -340,7 +347,7 @@ func NewGomegaWithT(t types.GomegaTestingT) *GomegaWithT {
 
 //See documentation for Expect
 func (g *GomegaWithT) Expect(actual interface{}, extra ...interface{}) GomegaAssertion {
-	return assertion.New(actual, testingtsupport.BuildTestingTGomegaFailHandler(g.t), 0, extra...)
+	return assertion.New(actual, testingtsupport.BuildTestingTGomegaFailWrapper(g.t), 0, extra...)
 }
 
 //See documentation for Eventually
@@ -353,7 +360,7 @@ func (g *GomegaWithT) Eventually(actual interface{}, intervals ...interface{}) G
 	if len(intervals) > 1 {
 		pollingInterval = toDuration(intervals[1])
 	}
-	return asyncassertion.New(asyncassertion.AsyncAssertionTypeEventually, actual, testingtsupport.BuildTestingTGomegaFailHandler(g.t), timeoutInterval, pollingInterval, 0)
+	return asyncassertion.New(asyncassertion.AsyncAssertionTypeEventually, actual, testingtsupport.BuildTestingTGomegaFailWrapper(g.t), timeoutInterval, pollingInterval, 0)
 }
 
 //See documentation for Consistently
@@ -366,7 +373,7 @@ func (g *GomegaWithT) Consistently(actual interface{}, intervals ...interface{})
 	if len(intervals) > 1 {
 		pollingInterval = toDuration(intervals[1])
 	}
-	return asyncassertion.New(asyncassertion.AsyncAssertionTypeConsistently, actual, testingtsupport.BuildTestingTGomegaFailHandler(g.t), timeoutInterval, pollingInterval, 0)
+	return asyncassertion.New(asyncassertion.AsyncAssertionTypeConsistently, actual, testingtsupport.BuildTestingTGomegaFailWrapper(g.t), timeoutInterval, pollingInterval, 0)
 }
 
 func toDuration(input interface{}) time.Duration {

internal/assertion/assertion.go

@@ -9,37 +9,42 @@ import (
 
 type Assertion struct {
 	actualInput interface{}
-	fail        types.GomegaFailHandler
+	failWrapper *types.GomegaFailWrapper
 	offset      int
 	extra       []interface{}
 }
 
-func New(actualInput interface{}, fail types.GomegaFailHandler, offset int, extra ...interface{}) *Assertion {
+func New(actualInput interface{}, failWrapper *types.GomegaFailWrapper, offset int, extra ...interface{}) *Assertion {
 	return &Assertion{
 		actualInput: actualInput,
-		fail:        fail,
+		failWrapper: failWrapper,
 		offset:      offset,
 		extra:       extra,
 	}
 }
 
 func (assertion *Assertion) Should(matcher types.GomegaMatcher, optionalDescription ...interface{}) bool {
+	assertion.failWrapper.TWithHelper.Helper()
 	return assertion.vetExtras(optionalDescription...) && assertion.match(matcher, true, optionalDescription...)
 }
 
 func (assertion *Assertion) ShouldNot(matcher types.GomegaMatcher, optionalDescription ...interface{}) bool {
+	assertion.failWrapper.TWithHelper.Helper()
 	return assertion.vetExtras(optionalDescription...) && assertion.match(matcher, false, optionalDescription...)
 }
 
 func (assertion *Assertion) To(matcher types.GomegaMatcher, optionalDescription ...interface{}) bool {
+	assertion.failWrapper.TWithHelper.Helper()
 	return assertion.vetExtras(optionalDescription...) && assertion.match(matcher, true, optionalDescription...)
 }
 
 func (assertion *Assertion) ToNot(matcher types.GomegaMatcher, optionalDescription ...interface{}) bool {
+	assertion.failWrapper.TWithHelper.Helper()
 	return assertion.vetExtras(optionalDescription...) && assertion.match(matcher, false, optionalDescription...)
 }
 
 func (assertion *Assertion) NotTo(matcher types.GomegaMatcher, optionalDescription ...interface{}) bool {
+	assertion.failWrapper.TWithHelper.Helper()
 	return assertion.vetExtras(optionalDescription...) && assertion.match(matcher, false, optionalDescription...)
 }
 
@@ -55,8 +60,9 @@ func (assertion *Assertion) buildDescription(optionalDescription ...interface{})
 func (assertion *Assertion) match(matcher types.GomegaMatcher, desiredMatch bool, optionalDescription ...interface{}) bool {
 	matches, err := matcher.Match(assertion.actualInput)
 	description := assertion.buildDescription(optionalDescription...)
+	assertion.failWrapper.TWithHelper.Helper()
 	if err != nil {
-		assertion.fail(description+err.Error(), 2+assertion.offset)
+		assertion.failWrapper.Fail(description+err.Error(), 2+assertion.offset)
 		return false
 	}
 	if matches != desiredMatch {
@@ -66,7 +72,7 @@ func (assertion *Assertion) match(matcher types.GomegaMatcher, desiredMatch bool
 		} else {
 			message = matcher.NegatedFailureMessage(assertion.actualInput)
 		}
-		assertion.fail(description+message, 2+assertion.offset)
+		assertion.failWrapper.Fail(description+message, 2+assertion.offset)
 		return false
 	}
 
@@ -80,7 +86,8 @@ func (assertion *Assertion) vetExtras(optionalDescription ...interface{}) bool {
 	}
 
 	description := assertion.buildDescription(optionalDescription...)
-	assertion.fail(description+message, 2+assertion.offset)
+	assertion.failWrapper.TWithHelper.Helper()
+	assertion.failWrapper.Fail(description+message, 2+assertion.offset)
 	return false
 }
 

internal/assertion/assertion_test.go

@@ -3,10 +3,13 @@ package assertion_test
 import (
 	"errors"
 
+	"github.com/onsi/gomega/internal/testingtsupport"
+
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	. "github.com/onsi/gomega/internal/assertion"
 	"github.com/onsi/gomega/internal/fakematcher"
+	"github.com/onsi/gomega/types"
 )
 
 var _ = Describe("Assertion", func() {
@@ -19,18 +22,21 @@ var _ = Describe("Assertion", func() {
 
 	input := "The thing I'm testing"
 
-	var fakeFailHandler = func(message string, callerSkip ...int) {
-		failureMessage = message
-		if len(callerSkip) == 1 {
-			failureCallerSkip = callerSkip[0]
-		}
+	var fakeFailWrapper = &types.GomegaFailWrapper{
+		Fail: func(message string, callerSkip ...int) {
+			failureMessage = message
+			if len(callerSkip) == 1 {
+				failureCallerSkip = callerSkip[0]
+			}
+		},
+		TWithHelper: testingtsupport.EmptyTWithHelper{},
 	}
 
 	BeforeEach(func() {
 		matcher = &fakematcher.FakeMatcher{}
 		failureMessage = ""
 		failureCallerSkip = 0
-		a = New(input, fakeFailHandler, 1)
+		a = New(input, fakeFailWrapper, 1)
 	})
 
 	Context("when called", func() {
@@ -186,7 +192,7 @@ var _ = Describe("Assertion", func() {
 				matcher.ErrToReturn = nil
 
 				var typedNil []string
-				a = New(input, fakeFailHandler, 1, 0, nil, typedNil)
+				a = New(input, fakeFailWrapper, 1, 0, nil, typedNil)
 
 				result := a.Should(matcher)
 				Expect(result).Should(BeTrue())
@@ -201,32 +207,32 @@ var _ = Describe("Assertion", func() {
 				matcher.MatchesToReturn = false
 				matcher.ErrToReturn = nil
 
-				a = New(input, fakeFailHandler, 1, errors.New("foo"))
+				a = New(input, fakeFailWrapper, 1, errors.New("foo"))
 				result := a.Should(matcher)
 				Expect(result).Should(BeFalse())
 				Expect(matcher.ReceivedActual).Should(BeZero(), "The matcher doesn't even get called")
 				Expect(failureMessage).Should(ContainSubstring("foo"))
 				failureMessage = ""
 
-				a = New(input, fakeFailHandler, 1, nil, 1)
+				a = New(input, fakeFailWrapper, 1, nil, 1)
 				result = a.ShouldNot(matcher)
 				Expect(result).Should(BeFalse())
 				Expect(failureMessage).Should(ContainSubstring("1"))
 				failureMessage = ""
 
-				a = New(input, fakeFailHandler, 1, nil, 0, []string{"foo"})
+				a = New(input, fakeFailWrapper, 1, nil, 0, []string{"foo"})
 				result = a.To(matcher)
 				Expect(result).Should(BeFalse())
 				Expect(failureMessage).Should(ContainSubstring("foo"))
 				failureMessage = ""
 
-				a = New(input, fakeFailHandler, 1, nil, 0, []string{"foo"})
+				a = New(input, fakeFailWrapper, 1, nil, 0, []string{"foo"})
 				result = a.ToNot(matcher)
 				Expect(result).Should(BeFalse())
 				Expect(failureMessage).Should(ContainSubstring("foo"))
 				failureMessage = ""
 
-				a = New(input, fakeFailHandler, 1, nil, 0, []string{"foo"})
+				a = New(input, fakeFailWrapper, 1, nil, 0, []string{"foo"})
 				result = a.NotTo(matcher)
 				Expect(result).Should(BeFalse())
 				Expect(failureMessage).Should(ContainSubstring("foo"))

internal/asyncassertion/async_assertion.go

@@ -22,11 +22,11 @@ type AsyncAssertion struct {
 	actualInput     interface{}
 	timeoutInterval time.Duration
 	pollingInterval time.Duration
-	fail            types.GomegaFailHandler
+	failWrapper     *types.GomegaFailWrapper
 	offset          int
 }
 
-func New(asyncType AsyncAssertionType, actualInput interface{}, fail types.GomegaFailHandler, timeoutInterval time.Duration, pollingInterval time.Duration, offset int) *AsyncAssertion {
+func New(asyncType AsyncAssertionType, actualInput interface{}, failWrapper *types.GomegaFailWrapper, timeoutInterval time.Duration, pollingInterval time.Duration, offset int) *AsyncAssertion {
 	actualType := reflect.TypeOf(actualInput)
 	if actualType.Kind() == reflect.Func {
 		if actualType.NumIn() != 0 || actualType.NumOut() == 0 {
@@ -37,18 +37,20 @@ func New(asyncType AsyncAssertionType, actualInput interface{}, fail types.Gomeg
 	return &AsyncAssertion{
 		asyncType:       asyncType,
 		actualInput:     actualInput,
-		fail:            fail,
+		failWrapper:     failWrapper,
 		timeoutInterval: timeoutInterval,
 		pollingInterval: pollingInterval,
 		offset:          offset,
 	}
 }
 
 func (assertion *AsyncAssertion) Should(matcher types.GomegaMatcher, optionalDescription ...interface{}) bool {
+	assertion.failWrapper.TWithHelper.Helper()
 	return assertion.match(matcher, true, optionalDescription...)
 }
 
 func (assertion *AsyncAssertion) ShouldNot(matcher types.GomegaMatcher, optionalDescription ...interface{}) bool {
+	assertion.failWrapper.TWithHelper.Helper()
 	return assertion.match(matcher, false, optionalDescription...)
 }
 
@@ -110,6 +112,8 @@ func (assertion *AsyncAssertion) match(matcher types.GomegaMatcher, desiredMatch
 		matches, err = matcher.Match(value)
 	}
 
+	assertion.failWrapper.TWithHelper.Helper()
+
 	fail := func(preamble string) {
 		errMsg := ""
 		message := ""
@@ -122,7 +126,8 @@ func (assertion *AsyncAssertion) match(matcher types.GomegaMatcher, desiredMatch
 				message = matcher.NegatedFailureMessage(value)
 			}
 		}
-		assertion.fail(fmt.Sprintf("%s after %.3fs.\n%s%s%s", preamble, time.Since(timer).Seconds(), description, message, errMsg), 3+assertion.offset)
+		assertion.failWrapper.TWithHelper.Helper()
+		assertion.failWrapper.Fail(fmt.Sprintf("%s after %.3fs.\n%s%s%s", preamble, time.Since(timer).Seconds(), description, message, errMsg), 3+assertion.offset)
 	}
 
 	if assertion.asyncType == AsyncAssertionTypeEventually {