Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Labels (and label-filtering) on BeforeSuite/BeforeAll/BeforeEach #1119

Open
tzvatot opened this issue Jan 19, 2023 · 8 comments
Open

Comments

@tzvatot
Copy link

tzvatot commented Jan 19, 2023

There are some tests that are labeled, for example as "slow" test or "fast" test.
The initialization is done in BeforeSuite, for both types of tests.

It means that when running "fast" test, the initialization of the slow tests is also running.
We can move the slow test initialization to it's own file, but then, when we will run "slow" tests, it will still run any initialization in the BeforeSuite (i.e. all the fast tests initialization).

Generally speaking, initialization (done in BeforeSuite) can be related to group of tests, the same way as tests are labeled.

BeforeSuite (or any BeforeX) not supporting labels. So we end up with all the initialization running, but only part of it is actually needed per the labels used.

It would be helpful if the BeforeSuite will support the labels: the same way as tests are skipped, we can skip some of the initialization too.

Is there a workaround for this?

@onsi
Copy link
Owner

onsi commented Jan 20, 2023

Hey! Yes there is a workaround though it isn't currently very elegant. You can do something like this:

BeforeSuite(func() {
	//first - get the Ginkgo Suite Configuration
	suiteConfig, _ := GinkgoConfiguration()
	
	//suiteConfig.LabelFilter is a string containing the label filters that were passed in with `ginkgo --label-filter=X`  we can parse it with `types.ParseLabelFilter()` from `github.com/onsi/ginkgo/v2/types`
	labelFilter, err := types.ParseLabelFilter(suiteConfig.LabelFilter)
   Expect(err).NotTo(HaveOccurred())

	//labelFilter is now a function that takes []string (a slice of labels) and returns a bool if those labels pass the filter.  you could do:

	if labelFilter([]string{"slow"}) {
		// do slow setup
	}

	if labelFilter([]string{"fast"}) {
		// do fast setup
	}
})

One limitation of this approach, however, is that it won't work if you do something like ginkgo --label-filter="slow && steady" as only specs with []string{"slow", "steady"} will run and labelFilter([]string{"slow"}) will return false. I don't have a great answer for you there. You could try to just inspect suiteConfig.LabelFilter yourself manually (e.g. strings.Contains(suiteConfig.LabelFilter, "slow")) but there will be edge cases you'll need to think through.

@tzvatot
Copy link
Author

tzvatot commented Jan 23, 2023

Thanks @onsi .
Any chance to build this in the framework too?

@onsi
Copy link
Owner

onsi commented Jan 23, 2023

What I could add is a method to SuiteConfig that can match against a set of labels. That would turn the above into:

BeforeSuite(func() {
	suiteConfig, _ := GinkgoConfiguration()

	if suiteConfig.MatchesLabels("slow") {
		// do slow setup
	}

	if suiteConfig.MatchesLabels("fast") {
		// do fast setup
	}
})

Or possibly even add a new top level function:

BeforeSuite(func() {
	if GinkgoMatchesLabels("slow") {
		// do slow setup
	}

	if GinkgoMatchesLabels("fast") {
		// do fast setup
	}
})

but that would be the extent of the integration. Is that what you're imagining?

@tzvatot
Copy link
Author

tzvatot commented Jan 24, 2023

@onsi top level function looks good to me!

@onsi
Copy link
Owner

onsi commented Jan 30, 2023

hey @tzvatot here's what it ended up looking like:

  • You can get the current configured label filter via GinkgoLabelFilter()
  • You can test any set of labels against an arbitrary filter via Label("a", "b", "c").MatchesLabelFilter("filter")

Putting this together you can do:

BeforeSuite(func() {
	if matches, err := Label("slow").MatchesLabelFilter(GinkgoLabelFilter()); matches && err == nil {
		// do slow setup
	}

	if matches, err := Label("fast").MatchesLabelFilter(GinkgoLabelFilter()); matches && err == nil {
		// do fast setup
	}
})

the error handling is a bit noisy but the reality is that the label filter could fail to parse so the error is necessary.

@onsi
Copy link
Owner

onsi commented Jan 30, 2023

Actually, never mind - that design is dumb given the context. I've made it so that MatchesLabelFilter uses MustParseLabelFilter under the hood which panics if the filter is invalid. You'll be able to:

BeforeSuite(func() {
	if Label("slow").MatchesLabelFilter(GinkgoLabelFilter()) {
		// do slow setup
	}

	if Label("fast").MatchesLabelFilter(GinkgoLabelFilter()) {
		// do fast setup
	}
})

when I cut the release

@onsi
Copy link
Owner

onsi commented Jan 30, 2023

This is now in Ginkgo 2.8.0

@tzvatot
Copy link
Author

tzvatot commented Feb 2, 2023

Thanks @onsi !

malandis added a commit to momentohq/client-sdk-go that referenced this issue Aug 22, 2024
Previously we matched on the labels of the current spec as opposed to
the label filter passed in from the CLI. This commit uses the correct
syntax as per ginkgo:

onsi/ginkgo#1119 (comment)
malandis added a commit to momentohq/client-sdk-go that referenced this issue Aug 22, 2024
Previously we matched on the labels of the current spec as opposed to
the label filter passed in from the CLI. This commit uses the correct
syntax as per ginkgo:

onsi/ginkgo#1119 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants