Add side input for golden file tests #203

llamallamaduck · 2022-01-26T15:46:57Z

What this PR does / why we need it:
This PR adds a possibility to provide a *.side_in.yaml file that contains resources that should exist before the resource defined in *.in.yaml is applied, when running golden file tests. These side input resources are going to be created before running a test case, and cleaned up at the end of it. The resources will be accessible via Get and List. Note that the existing functionality has not been changed.

Context from which the need for this change came up:
We have 3 CRDs: Event(E), Component(C) and EventComponentDeployment(ECD). When an E is created, for every existing C we create an ECD. Every ECD has a reference to E and C, and upon creation it further creates other resources based on whats inside E and C. To be able to test E controller and ECD controller with golden file tests, we needed a way easily create surrounding resources (Cs for testing E controller, and Es and Cs for testing ECD controller). This change works well for this use case.

Note for reviewers:
The second commit in this PR makes the examples run with local library (to include changes in this PR), which required running go mod tidy, which resulted in a lot of changes. If you prefer me to remove this commit, and update the tests in example later/in a separate PR, let me know. The gist of this change is in the first commit.

linux-foundation-easycla · 2022-01-26T15:47:00Z

❌ The commit (5ab7463) is missing the User's ID, preventing the EasyCLA check. Consult GitHub Help to resolve.For further assistance with EasyCLA, please submit a support request ticket.

k8s-ci-robot · 2022-01-26T15:47:03Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: login-issues@jira.linuxfoundation.org

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/test-infra repository. I understand the commands that are listed here.

k8s-ci-robot · 2022-01-26T15:47:05Z

Welcome @llamallamaduck!

It looks like this is your first PR to kubernetes-sigs/kubebuilder-declarative-pattern 🎉. 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-declarative-pattern 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 · 2022-01-26T15:47:05Z

Hi @llamallamaduck. 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/test-infra repository.

llamallamaduck · 2022-01-26T15:56:31Z

pkg/test/mocks/client.go

+	nonListKind := gvkUnprocessed.Kind
+	nonListKind = nonListKind[:len(nonListKind)-4]
+	gvk := schema.GroupVersionKind{gvkUnprocessed.Group, gvkUnprocessed.Version, nonListKind}
+	gvr, _ := meta.UnsafeGuessKindToResource(gvk)


Not super happy with the way of figuring out gvk and gvr here - any suggestions on how to make it nicer?

+1 for this, I wonder if folks from api-server will be more aware about this.

I'll take a look but I don't want to hold up this PR (sorry for the delay already!) :-)

linux-foundation-easycla · 2022-01-27T12:46:06Z

❌ The commit (d702149) is missing the User's ID, preventing the EasyCLA check. Consult GitHub Help to resolve.For further assistance with EasyCLA, please submit a support request ticket.

linux-foundation-easycla · 2022-01-27T16:02:20Z

The committers are authorized under a signed CLA.

✅ Maja Kontrec (a553f84)

atoato88 · 2022-01-31T01:58:09Z

@llamallamaduck As k8s-ci-bot says, could you sign CLA?
Because this PR has passed EasyCLA check, but "cla/linuxfoundation" has not passed.
I think both checks are needed for merging this PR.

atoato88 · 2022-01-31T01:59:07Z

/check-cla

atoato88 · 2022-01-31T02:14:29Z

/ok-to-test
It looks good for testing. 🚀

llamallamaduck · 2022-02-01T14:23:31Z

/test pull-declarative-test

llamallamaduck · 2022-02-01T14:31:28Z

/test pull-declarative-test

llamallamaduck · 2022-02-01T16:04:48Z

@llamallamaduck As k8s-ci-bot says, could you sign CLA? Because this PR has passed EasyCLA check, but "cla/linuxfoundation" has not passed. I think both checks are needed for merging this PR.

Yeah, I've done everything as stated in instructions and still no luck. I've opened a support ticket to sort it out.

Adds a posibility to provide a *.side_in.yaml file, that contains resources that should exist before the resource defined in `*.in.yaml` is applied. These side input resources are going to be created before running a testcase, and cleaned up at the end of it. The resources will be accessible via Get and List. Note that the existing functionality has not been changed.

WillsonHG · 2022-02-03T22:11:57Z

/check-cla

irvifa · 2022-02-08T17:04:18Z

/cc @kubernetes-sigs/kubernetes-sig-api-machinery-reviewers

irvifa · 2022-02-08T17:05:50Z

/cc @kubernetes-sigs/kubebuilder-declarative-pattern-admins

irvifa · 2022-02-08T17:06:50Z

docs/addon/walkthrough/tests.md

@@ -22,6 +22,11 @@ The bulk of these tests are implemented in `pkg/tests` via the `StandardValidato
 The test scans a `tests` directory for `*.in.yaml` files, runs the operator for each 
 of them, and compares the output to `*.out.yaml` files.

+It is also possible to provide `*.side_in.yaml` file that contains resources that
+should exist before the resource defined in `*.in.yaml` is applied. These resources
+are going to be created before running a test, and cleaned up upon finishing. They


Suggested change

are going to be created before running a test, and cleaned up upon finishing. They

are going to be created before running a test, and cleaned up upon finishing. It

irvifa · 2022-02-08T17:20:49Z

pkg/test/mocks/client.go

+	nonListKind := gvkUnprocessed.Kind
+	nonListKind = nonListKind[:len(nonListKind)-4]
+	gvk := schema.GroupVersionKind{gvkUnprocessed.Group, gvkUnprocessed.Version, nonListKind}
+	gvr, _ := meta.UnsafeGuessKindToResource(gvk)


+1 for this, I wonder if folks from api-server will be more aware about this.

atoato88 · 2022-02-28T00:58:15Z

/lgtm

justinsb · 2022-03-17T20:27:57Z

pkg/test/golden/validator.go

+		sideInputPath := strings.Replace(p, ".in.yaml", ".side_in.yaml", -1)
+		sideInputRead, err := os.ReadFile(sideInputPath)
+		if err != nil {
+			t.Logf("Could not read side input file %s: %v, skipping", sideInputPath, err)


Nit: I like to check that the error is indeed os.IsNotFound(err) (I'm assuming that's what we're expecting), and avoid the log message - and possibly avoid covering up another error (like e.g. a permissions error)

justinsb · 2022-03-17T20:29:27Z

pkg/test/mocks/client.go

+	nonListKind := gvkUnprocessed.Kind
+	nonListKind = nonListKind[:len(nonListKind)-4]
+	gvk := schema.GroupVersionKind{gvkUnprocessed.Group, gvkUnprocessed.Version, nonListKind}
+	gvr, _ := meta.UnsafeGuessKindToResource(gvk)


I'll take a look but I don't want to hold up this PR (sorry for the delay already!) :-)

justinsb · 2022-03-17T20:31:18Z

Thanks @llamallamaduck - this looks good. I'll take a look at the list thing separately - there should be a helper for this, but I don't know of one immediately.

/approve
/lgtm

k8s-ci-robot · 2022-03-17T20:31:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb, llamallamaduck

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [justinsb]

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 added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 26, 2022

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 26, 2022

k8s-ci-robot requested review from justinsb and pwittrock January 26, 2022 15:47

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jan 26, 2022

llamallamaduck commented Jan 26, 2022

View reviewed changes

llamallamaduck force-pushed the add-golden-side-input branch from 5ab7463 to 4d3227e Compare January 27, 2022 12:46

llamallamaduck force-pushed the add-golden-side-input branch 2 times, most recently from d702149 to a553f84 Compare January 27, 2022 16:02

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 31, 2022

llamallamaduck force-pushed the add-golden-side-input branch from 7c72cb2 to ba9744f Compare February 1, 2022 14:57

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 1, 2022

llamallamaduck force-pushed the add-golden-side-input branch from 0009745 to ed7f42e Compare February 1, 2022 16:57

llamallamaduck added 2 commits February 2, 2022 16:17

Use local library in examples, run go mod tidy

c8123e8

llamallamaduck force-pushed the add-golden-side-input branch from ed7f42e to c8123e8 Compare February 2, 2022 15:18

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Feb 3, 2022

WillsonHG mentioned this pull request Feb 3, 2022

Status not reporting or updating correctly on PR communitybridge/easycla#3378

Closed

k8s-ci-robot requested a review from a team February 8, 2022 17:05

irvifa reviewed Feb 8, 2022

View reviewed changes

k8s-ci-robot assigned atoato88 Feb 28, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 28, 2022

justinsb approved these changes Mar 17, 2022

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 17, 2022

k8s-ci-robot assigned justinsb Mar 17, 2022

k8s-ci-robot merged commit 96a66df into kubernetes-sigs:master Mar 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add side input for golden file tests #203

Add side input for golden file tests #203

llamallamaduck commented Jan 26, 2022 •

edited

Loading

linux-foundation-easycla bot commented Jan 26, 2022

k8s-ci-robot commented Jan 26, 2022

k8s-ci-robot commented Jan 26, 2022

k8s-ci-robot commented Jan 26, 2022

llamallamaduck Jan 26, 2022

irvifa Feb 8, 2022

justinsb Mar 17, 2022

linux-foundation-easycla bot commented Jan 27, 2022 •

edited

Loading

linux-foundation-easycla bot commented Jan 27, 2022 •

edited

Loading

atoato88 commented Jan 31, 2022

atoato88 commented Jan 31, 2022

atoato88 commented Jan 31, 2022

llamallamaduck commented Feb 1, 2022

llamallamaduck commented Feb 1, 2022

llamallamaduck commented Feb 1, 2022

WillsonHG commented Feb 3, 2022

irvifa commented Feb 8, 2022

irvifa commented Feb 8, 2022

irvifa Feb 8, 2022

irvifa Feb 8, 2022

atoato88 commented Feb 28, 2022

justinsb Mar 17, 2022

justinsb Mar 17, 2022

justinsb commented Mar 17, 2022

k8s-ci-robot commented Mar 17, 2022

	are going to be created before running a test, and cleaned up upon finishing. They
	are going to be created before running a test, and cleaned up upon finishing. It

Add side input for golden file tests #203

Add side input for golden file tests #203

Conversation

llamallamaduck commented Jan 26, 2022 • edited Loading

linux-foundation-easycla bot commented Jan 26, 2022

k8s-ci-robot commented Jan 26, 2022

k8s-ci-robot commented Jan 26, 2022

k8s-ci-robot commented Jan 26, 2022

llamallamaduck Jan 26, 2022

Choose a reason for hiding this comment

irvifa Feb 8, 2022

Choose a reason for hiding this comment

justinsb Mar 17, 2022

Choose a reason for hiding this comment

linux-foundation-easycla bot commented Jan 27, 2022 • edited Loading

linux-foundation-easycla bot commented Jan 27, 2022 • edited Loading

atoato88 commented Jan 31, 2022

atoato88 commented Jan 31, 2022

atoato88 commented Jan 31, 2022

llamallamaduck commented Feb 1, 2022

llamallamaduck commented Feb 1, 2022

llamallamaduck commented Feb 1, 2022

WillsonHG commented Feb 3, 2022

irvifa commented Feb 8, 2022

irvifa commented Feb 8, 2022

irvifa Feb 8, 2022

Choose a reason for hiding this comment

irvifa Feb 8, 2022

Choose a reason for hiding this comment

atoato88 commented Feb 28, 2022

justinsb Mar 17, 2022

Choose a reason for hiding this comment

justinsb Mar 17, 2022

Choose a reason for hiding this comment

justinsb commented Mar 17, 2022

k8s-ci-robot commented Mar 17, 2022

llamallamaduck commented Jan 26, 2022 •

edited

Loading

linux-foundation-easycla bot commented Jan 27, 2022 •

edited

Loading

linux-foundation-easycla bot commented Jan 27, 2022 •

edited

Loading