🌱 Added e2e tests for alpha generate command for scaffolded data

[manalilatkar]

• Added e2e tests for alpha generate command for scaffolded data under testdata
• made some minor changes in underlying testContext to enable creation of TestContext for scaffolded data and
  destruction of the regenerated directory
• added a makefile entry for running all these commands
• added a github workflow to run these tests on specific changes
• submitting PR as a solution to this issue Enhance Test for kubebuilder alpha generate Command #4508

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: manalilatkar / name: Manali Latkar (3d5d2eb)

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: manalilatkar
Once this PR has been reviewed and has the lgtm label, please assign varshaprasad96 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

Welcome @manalilatkar!

It looks like this is your first PR to kubernetes-sigs/kubebuilder 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubebuilder has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @manalilatkar. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[camilamacedo86]

The tests under test/e2e have already been executed in the prow, so we do not need a GitHub action or a new script for those.

[manalilatkar]

Removed file.

[camilamacedo86]

/ok-to-test

[camilamacedo86]

Hi @manalilatkar

It is nice; It seems that your changes passed the tests.
I will try properly review asap as possible, but also see that you need to sign: #4554 (comment) ( without it we cannot get it merged )

[manalilatkar]

Hi @manalilatkar

It is nice; It seems that your changes passed the tests. I will try properly review asap as possible, but also see that you need to sign: #4554 (comment) ( without it we cannot get it merged )

@camilamacedo86 Thanks. Working on the CLA. Have to get internal approval from my organization. Will try to finish it asap.

[camilamacedo86]

Hi @manalilatkar

Thank you a lot
Take your time to solve the SLA

[manalilatkar]

@camilamacedo86 My CLA is done. Can you please review and let me know if any changes need to be done?

[camilamacedo86]

We have already:

kubebuilder/test/e2e/utils/test_context.go

Lines 229 to 247 in de4c4c7

	func (t *TestContext) Destroy() {
	//nolint:gosec
	// if image name is not present or not provided skip execution of docker command
	if t.ImageName != "" {
	// Check white space from image name
	if len(strings.TrimSpace(t.ImageName)) == 0 {
	log.Println("Image not set, skip cleaning up of docker image")
	} else {
	cmd := exec.Command("docker", "rmi", "-f", t.ImageName)
	if _, err := t.Run(cmd); err != nil {
	warnError(err)
	}
	}
	}
	if err := os.RemoveAll(t.Dir); err != nil {
	warnError(err)
	}
	}

Could we not use what we have?
Then, due to the "output" dir being specific for those tests, do we call the removal on the tests specifically?

[camilamacedo86]

Why do we need to create the outputDir?
Can we not call the re-generation in the dir where the project is?
Then we also test it out?

[manalilatkar]

@camilamacedo86 Then should I copy the contents of the scaffolded project to the Kbc.Dir temp directory and let it destroy with the original method. Is that acceptable?
Is it ok if I use https://github.com/otiai10/copy to copy the contents of the scaffolded project or should I write a custom function for that?

[manalilatkar]

Creating the output Dir because I picked up the convention from the test/e2e/generate_test.go file:

kubebuilder/test/e2e/alphagenerate/generate_test.go

Line 55 in 74ef8af

projectFilePath = filepath.Join(projectOutputDir, "PROJECT")

[camilamacedo86]

Great work! 🥇🎉

Just a couple of small nits:

• We can simplify by not creating another directory.
• Running the tests will already create a temp directory stored at kbc.Dir.
• No need to create a new Destroy method since we already have one.

Otherwise, all good LGTM
Can you try to simplify by addressing those nits?
Then, I think after that we can get this one merged.
WDYT?

[camilamacedo86]

Because this change does not impact end users ( we are not changing the scaffolds ) we use :🌱:
To know more about see: https://github.com/kubernetes-sigs/kubebuilder/blob/master/CONTRIBUTING.md#pr-process

I hope that you do not mind, I updated the title accordingly.

[manalilatkar]

Thanks. Will keep in mind.

[manalilatkar]

Great work! 🥇🎉

Just a couple of small nits:

• We can simplify by not creating another directory.
• Running the tests will already create a temp directory stored at kbc.Dir.
• No need to create a new Destroy method since we already have one.

Otherwise, all good LGTM Can you try to simplify by addressing those nits? Then, I think after that we can get this one merged. WDYT?

@camilamacedo86 Thanks. Removed the DestroyOutputDir method. Still regenerating the project in the output directory in the testdata project directories as that is the convention used in the tests in est/e2e/generate_test.go file. If I regenerate the project in the same directory, there will be unwanted changes after running the alpha generate command. The easiest way to clean it up is to regenerate in the output directory which can be easily destroyed using kbc.Destroy(). Anyway, let me know if my new changes look good. No changes to any existing framework. Just 3 new files.