Add -tags flag to ginkgo generate command

[alegrey91]

Hi all,
Since we use a lot of build tags in tests with ginkgo, we would really appreciate adding the -tags flag in the ginkgo generate command.
The flag should allow the user to add the provided build tags to the generated file.

Expected behavior

Here's an example command:

ginkgo generate -tags "e2e,!integration" new_test.go

This should generate the following file:

//go:build e2e,!integration

package e2e_test

import (
        . "github.com/onsi/ginkgo"
        . "github.com/onsi/gomega"
)

var _ = Describe("File", func() {

})

If you like the idea, I can take care of the development :)

[onsi]

hey - happy to pull this in if you submit a PR. out of curiosity - what are the reasons for preferring build tags over labels?

[alegrey91]

hey - happy to pull this in if you submit a PR.

Great, thanks :)

out of curiosity - what are the reasons for preferring build tags over labels?

To be honest, I didn't know about labels (good to know 😅). I started using ginkgo a few weeks ago, so I'm not experienced with it.
Anyway, I don't know if it really fits with our needs. I try to elaborate more: We are tagging files that only contain variables filled with values of a specific provider. So usually the test is shared between different providers. What really changes during the build is the value of the variables used in the test.
It was thought to make it extensible, so the more providers you want to support, the more tagged files you need. You can take a look here at what I'm talking about:

• code for aks provider
• code for kind provider
• shared test

Basically, we are using tags to select the values, not the tests. That's more or less the idea we had.

[onsi]

hey @alegrey91 your approach works and makes sense to me - some of this is naturally subjective as there are multiple ways to solve for this sort of thing. If you never expect to run against multiple targets at once (e.g. in parallel) then your approach of doing it at compile time makes sense.

I would, personally, opt for a run-time solution using labels as it would allow me to potentially run a single suite against multiple infrastructure targets in parallel simultaneously.

if you're interested i can share an example of how to wire things up.

either way, though, i'd be happy to accept the PR you're proposing.

[alegrey91]

hey @alegrey91 your approach works and makes sense to me - some of this is naturally subjective as there are multiple ways to solve for this sort of thing. If you never expect to run against multiple targets at once (e.g. in parallel) then your approach of doing it at compile time makes sense.

Thanks :)
For now, our parallelism is given by using the github-action matrices. So, yes, we do this in some way.

I would, personally, opt for a run-time solution using labels as it would allow me to potentially run a single suite against multiple infrastructure targets in parallel simultaneously.

if you're interested i can share an example of how to wire things up.

Sure I'm interested! We are going to release the e2e tests in few days, but this could be helpful for future improvements and also for my personal knowledge :)

either way, though, i'd be happy to accept the PR you're proposing.

Yeh, I would like to contribute anyway. We are going to use this feature for some time at least.

[onsi]

Circling back to this - there are a variety of ways to approach this to enable multiple providers and/or runtime-selection of providers vs compile-time. Here's one that comes to mind:

const AKS = "aks"
const KIND = "kind"
var PROVIDERS = []string{AKS, KIND}
var KUBELET_INFOS = map[string]*sensor.KubeletInfo{
	AKS: &sensor.KubeletInfo{...},
	KIND: &sensor.KubeletInfo{...},
}

var _ = Describe("Kubeletinfo", func() {
	var (
		res     *http.Response
		err     error
		resBody []byte
	)

	for _, provider := range PROVIDERS {
		kubeletInfo := KUBELET_INFOS[provider] 

		Context("testing /kubeletinfo endpoint", Label(provider), Ordered, func() {
			It("should respond to a GET request", func() {
				requestURL := url + "/kubeletinfo"
				res, err = http.Get(requestURL)
				Expect(err).ToNot(HaveOccurred())
			})
			It("should return a 200 status code", func() {
				Expect(res.StatusCode).To(BeEquivalentTo(200))
			})
			It("should return the expected value of KubeletInfo", func() {
				resultBody := &sensor.KubeletInfo{}
	
				resBody, err = ioutil.ReadAll(res.Body)
				Expect(err).ToNot(HaveOccurred())
	
				err = json.Unmarshal(resBody, resultBody)
				Expect(err).ToNot(HaveOccurred())
	
				Expect(resultBody.ServiceFiles).
					To(Equal(kubeletInfo.ServiceFiles))
				Expect(resultBody.KubeConfigFile.Path).
					To(Equal(kubeletInfo.KubeConfigFile.Path))
				Expect(resultBody.ClientCAFile).
					To(Equal(kubeletInfo.ClientCAFile))
			})
		})	
	}
})

there could be several other approaches no doubt, as well. Now to run all providers you just do ginkgo. To run just one provier you can do ginkgo --label-filter=kind; to run two providers: ginkgo -label-filter=kind||aks.

One important thing, though. I noticed that the test you linked to assumes the tests run in order. This is actually only guaranteed if you are running in series and haven't used --randomize-all. I recommend reading [this section] of the documentation to learn about why Ginkgo assumes your specs are actually independent. And then this section to learn about randomization and finally this section to learn about parallelization. I added the Ordered decorator to your example to ensure correctness but I recommend reading those docs to get a deeper mental model for Ginkgo, especially if you're planning to build a large comple e2e suite with it.

The independent-spec parallel friendly version of your test would look like:

Context("testing /kubeletinfo endpoint", func() {
	var (
		res     *http.Response
		err     error
		resBody []byte
	)

	BeforeEach(func() {
		requestURL := url + "/kubeletinfo"
		res, err = http.Get(requestURL)
		Expect(err).ToNot(HaveOccurred())
	})
	
	It("should return a 200 status code", func() {
		Expect(res.StatusCode).To(BeEquivalentTo(200))
	})
	
	It("should return the expected value of KubeletInfo", func() {
		resultBody := &sensor.KubeletInfo{}

		resBody, err = ioutil.ReadAll(res.Body)
		Expect(err).ToNot(HaveOccurred())

		err = json.Unmarshal(resBody, resultBody)
		Expect(err).ToNot(HaveOccurred())

		Expect(resultBody.ServiceFiles).
			To(Equal(kubeletInfo.ServiceFiles))
		Expect(resultBody.KubeConfigFile.Path).
			To(Equal(kubeletInfo.KubeConfigFile.Path))
		Expect(resultBody.ClientCAFile).
			To(Equal(kubeletInfo.ClientCAFile))
	})
})

[alegrey91]

Wow! Thanks for the deep explanation, @onsi!!
I'll try to assimilate as much as possible to improve my skills :)