search

kubernetes-sigs / kubebuilder-declarative-pattern

A toolkit for building declarative operators with kubebuilder
Apache License 2.0
254 stars 84 forks source link

Add unit test for declarative/labels #208

Closed atoato88 closed 2 years ago

atoato88 commented 2 years ago

What this PR does / why we need it: This PR adds unit tests for pkg/patterns/declarative/labels_test.go.

Which issue(s) this PR fixes: NONE

Special notes for your reviewer: NONE

Additional documentation: I would like to add unit tests for package pkg/patterns/declarative because of this package has many manipuration logics of this repository.

justinsb commented 2 years ago

/approve /lgtm

k8s-ci-robot commented 2 years ago

[APPROVALNOTIFIER] This PR is APPROVED

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

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](https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern/blob/master/OWNERS)~~ [atoato88,justinsb] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
justinsb commented 2 years ago

Thanks @atoato88 ... we could also consider creating some golden tests (e.g. https://ieftimov.com/posts/testing-in-go-golden-files/ ) that test whole operator functionality, and then do a few tests with the various options. But this lgtm!

atoato88 commented 2 years ago

Thank you to LGTM to this PR and give idea about testing!! :smile: I'll check the URL.

atoato88 commented 2 years ago

@justinsb Thank you to let me know the article about golden file test. I agree with this plan, so should we implement operator (maybe dummy operator?) for testing functionalities for this repo ? Or, we should be versioning this repo before this?

justinsb commented 2 years ago

I created the release-0.11 branch as a first step towards versioning! But creating the branch doesn't really commit us to anything :-)

I don't think tests should impact the release versioning (unless they find problems!). I think we should use versioning for breaking changes, and ideally we would backport fixes at least to the current released version while we're developing the next version. So if we start thinking of master as the upcoming 0.12.0 release, then we should backport any bugfixes to the release-0.11 branch; we would expect operators that want stability to use release-0.11 until we release release-0.12.

Because it's a library, "releasing" just means tagging I think.

And yes, in terms of the testing I was thinking we should create a test operator and maybe expose a few hooks so that it's easy to create derived tests testing different configurations. And we should run it in our e2e against the k8s testclient. Similar to the work you did to create golden tests like this one - I'm wondering if we can use that approach more widely and for more configurations. But it's just a thought, if you want to explore it that's wonderful, otherwise hopefully I'll be able to explore it!

atoato88 commented 2 years ago

Thank you to detailed comment. Yes, I agree with your plan, and we need test operator like guestbook-operator and tests for that. It's good to create test operator at first, and expand cover range about test functionalities, I think.