onsi / ginkgo

A Modern Testing Framework for Go
http://onsi.github.io/ginkgo/
MIT License
8.12k stars 644 forks source link

Ginkgohelper does not work well with Eventually #1322

Closed isibeni closed 7 months ago

isibeni commented 7 months ago

I have a rather specific use-case where GinkgoHelper does not work as 'expected'.

I'm using a helper function that is retrieving some data. Within that function I also use Expect(err).NotTo(HaveOccurred()) and therefore I'm also using GinkgoHelper().

When the mentioned Expect fails the line, from where the helper function is called, is mostly determined correctly. However this does not work when using the function within an Eventually function together with WithArguments.

To better demonstrate this I provided some sample:

package ginkgohelper_test

import (
    "errors"

    . "github.com/onsi/ginkgo/v2"
    . "github.com/onsi/gomega"
)

func GetData(x string) (string, error) {
    return x, errors.New(x)
}

func FooHelper(x string) string {
    GinkgoHelper()

    data, err := GetData(x)           // Some data retrieval
    Expect(err).NotTo(HaveOccurred()) // Ensure that no error occurred
    return data
}

var _ = Describe("GinkgoHelper", func() {
    It("helper works as expected with expect", func() {
        Expect(FooHelper("foo")).Should(Equal("foo"))
    })

    It("helper works as expected with eventually", func() {
        Eventually(func() string {
            return FooHelper("foo")
        }).Should(Equal("foo"))
    })

    It("helper does not work as 'expected' with eventually.WithArguments", func() {
        Eventually(FooHelper).WithArguments("foo").Should(Equal("foo"))
    })
})

For the first two scenarios everything works as expected. For the later one the following line will be determined: /usr/local/Cellar/go/1.21.4/libexec/src/reflect/value.go:596. See:

Summarizing 3 Failures:
  [FAIL] GinkgoHelper [It] helper works as expected with expect
  /example/ginkgohelper/ginkgohelper_test.go:24 # correct line
  [FAIL] GinkgoHelper [It] helper works as expected with eventually
  /example/ginkgohelper/ginkgohelper_test.go:29 # correct line
  [FAIL] GinkgoHelper [It] helper does not work as 'expected' with eventually.WithArguments
  /usr/local/Cellar/go/1.21.4/libexec/src/reflect/value.go:596 # wrong line

Ran 3 of 3 Specs in 0.001 seconds
FAIL! -- 0 Passed | 3 Failed | 0 Pending | 0 Skipped

For the last scenario I would get the proper line if I would use ExpectWithOffset(6,.. instead of GinkgoHelper.

I would not consider this as bug, but I would be curious if there is a way to get this working.

Any help is appreciated. Thx in advance

PS:

I have the following stack (when using debug.PrintStacktrace):

runtime/debug.Stack()
        /usr/local/Cellar/go/1.21.4/libexec/src/runtime/debug/stack.go:24 +0x5e
runtime/debug.PrintStack()
        /usr/local/Cellar/go/1.21.4/libexec/src/runtime/debug/stack.go:16 +0x13
example.com/ginkgohelper_test.FooHelper({0x13eb383, 0x3})
        /example/ginkgohelper/ginkgohelper_test.go:19 +0x86
reflect.Value.call({0x136e400?, 0x14294a0?, 0x100dba5?}, {0x13eb7c9, 0x4}, {0xc000012a68, 0x1, 0x14981a0?})
        /usr/local/Cellar/go/1.21.4/libexec/src/reflect/value.go:596 +0xce7
reflect.Value.Call({0x136e400?, 0x14294a0?, 0x14981a0?}, {0xc000012a68?, 0x1d230b52fad02?, 0xc000085cf0?})
        /usr/local/Cellar/go/1.21.4/libexec/src/reflect/value.go:380 +0xb9
github.com/onsi/gomega/internal.(*AsyncAssertion).buildActualPoller.func3()
        /example/vendor/github.com/onsi/gomega/internal/async_assertion.go:325 +0x11f
github.com/onsi/gomega/internal.(*AsyncAssertion).match(0xc00023ae00, {0x149c180?, 0xc00023e4c0}, 0x1, {0x0, 0x0, 0x0})
        /example/vendor/github.com/onsi/gomega/internal/async_assertion.go:398 +0x179
github.com/onsi/gomega/internal.(*AsyncAssertion).Should(0xc00023ae00, {0x149c180, 0xc00023e4c0}, {0x0, 0x0, 0x0})
        /example/vendor/github.com/onsi/gomega/internal/async_assertion.go:145 +0x86
example.com/ginkgohelper_test.glob..func1.3()
        /example/ginkgohelper/ginkgohelper_test.go:36 +0xc2
github.com/onsi/ginkgo/v2/internal.extractBodyFunction.func3({0x0, 0x0})
        /example/vendor/github.com/onsi/ginkgo/v2/internal/node.go:463 +0x13
github.com/onsi/ginkgo/v2/internal.(*Suite).runNode.func3()
        /example/vendor/github.com/onsi/ginkgo/v2/internal/suite.go:889 +0x8d
created by github.com/onsi/ginkgo/v2/internal.(*Suite).runNode in goroutine 4
        /example/vendor/github.com/onsi/ginkgo/v2/internal/suite.go:876 +0xd7b
onsi commented 7 months ago

hey there,

thanks for the clear writuep and reproducer. unfortunately there's a more subtle issue at play. Calls to Expect immediately call Gomega's fail handler when they fail and so making an assertion in a function that is passed to Eventually that fails at any point when polled will always cause the Eventually to fail. You can try this to see:

func GetData(x string) (string, error) {
    return x, errors.New(x)
}

var n = 0

func FooHelper(x string) string {
    GinkgoHelper()

    data, err := GetData(x) // Some data retrieval
    n++
    if n < 3 {
        Expect(err).NotTo(HaveOccurred()) // Ensure that no error occurred
    }
    return data
}

var _ = Describe("GinkgoHelper", func() {
    It("helper never actually succeeds", func() {
        Eventually(func() string {
            return FooHelper("foo")
        }).Should(Equal("foo"))
    })
})

here the first few invocations of FooHelper will fail. The helper will eventually succeed but the damage has been done as Expect has already called Gomega's fail handler which marks the test as failed.

The docs cover how to implement helper functions that are passed to Eventually that need to make assertions here.

I believe that doing that will also resolve the issue you are reporting.

I appreciate that this is a subtle gotcha - it's an unfortuante side effect of Gomega's usage of global variables to avoid having to pass references to a Gomega instance everywhere.

isibeni commented 7 months ago

Thanks. Explanation and link to the docs helped a lot. Totally missed that part.