nunnatsa / ginkgolinter

golang linter for ginkgo and gomega
MIT License
26 stars 6 forks source link

[Enhancement] Check for bare Expect inside Eventually #85

Closed mnencia closed 1 year ago

mnencia commented 1 year ago

Consider the following code:

Eventually(func() string {
    myVal, err := mymodule.GetMyVal()
    Expect(err).ToNot(HaveOccurred())

    myResult, err := myVal.DoSomething()
    Expect(err).ToNot(HaveOccurred())

    return myResult
}, 60).Should(BeEquivalentTo(expectedResult))

You could expect that in case of any function returns an error, the block is retried, but it is not.

What you usually want to do is:

Eventually(func(g Gomega) string {
    myVal, err := mymodule.GetMyVal()
    g.Expect(err).ToNot(HaveOccurred())

    myResult, err := myVal.DoSomething()
    g.Expect(err).ToNot(HaveOccurred())

    return myResult
}, 60).Should(BeEquivalentTo(expectedResult))

or

Eventually(func() (string, error) {
    myVal, err := mymodule.GetMyVal()
    if err != nil {
        return "", err
    }

    myResult, err := myVal.DoSomething()
    if err != nil {
        return "", err
    }

    return myResult, nil
}, 60).Should(BeEquivalentTo(expectedResult))

I think highlighting the presence of a bare Expect inside an Eventually call would be useful.

nunnatsa commented 1 year ago

Thaks for your suggestion, @mnencia.

The thing is, that bare Except is valid within Eventually. This is the way to exit the Eventually loop in case of a non recoverable error.

I'm not dute I know how to automatically distinguish between the two use cases. Do you have any idea?

nunnatsa commented 1 year ago

@mnencia, if you don't have any objections, I'll close this issue. Please reopen if you have any idea.