onsi / ginkgo

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

Focused specs are disregarded when using label filtering #1221

Closed mokiat closed 1 year ago

mokiat commented 1 year ago

Hi,

When using --label-filter, any FIts are treated as regular Its and ginkgo executes all of them. Furthermore, it exits with a regular 0 exit code, presuming all tests pass.

This can create issues when running the tests locally wihout filtering and splitting them in a CI with filtering. I have experienced cases where code with FIts gets pushed to master as the developer has forgotten to remove the focusing and the CI testing in the MR does not catch it due to the aforementioned behavior. The next developer to clone the code locally is surprised to find that only one or two specs are executed.

Following is example code to test this:

package ginkgo_issue_test

import (
    "testing"

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

func TestGinkgoIssue(t *testing.T) {
    RegisterFailHandler(Fail)
    RunSpecs(t, "GinkgoIssue Suite", Label("api"))
}

var _ = Describe("example", func() {
    FIt("first", func() {
        Expect(true).To(BeTrue())
    })

    It("second", func() {
        Expect(false).To(BeFalse())
    })
})

Try running the above code with ginkgo (only a single spec is run) and then with ginkgo --label-filter='api' (all specs are run).

onsi commented 1 year ago

hey this came up on slack recently as well. it’s technically behaving as intended but i think you are correct that this behavior isn’t great - so we should change it!

how about:

mokiat commented 1 year ago

Thanks for the quick response to this.

Yes, having both of them seems like the logical way forward to me.

Usually we use label filters to pick a subset of tests (API, DB or Smoke, Acceptance, Unit). In such context, having an FIt means for example hey, I'd like to focus on that specific Acceptance test. So FIt is always ANDed with all other filters would work well here.

Correct me if I am wrong, but if I recall correctly, the non-zero code for focused tests was introduced with the exact purpose of preventing focused tests from passing through automated tests in PRs / MRs and reaching the stable branch. So FIt always results in a non-zero exit code even if there are other filters. would cover that case as well.

onsi commented 1 year ago

hey @mokiat this is now fixed in 2.11.0 - give it a go and let me know!

mokiat commented 1 year ago

@onsi , I just tried v2.11.0 and it works like a charm. Thank you.

Closing this issue as resolved.