🌱 (CLI) Add unit tests for cli alpha generate.go #5074

mayuka-c · 2025-09-03T04:40:03Z

Updates: #4925

Improve the coverage by adding UTs for cli alpha generate.go.
This is the continuation of my other UT PRs to improve coverage :)

Also had to make slight modification (after rebasing with main branch) to use dependency injection for mocking executable because of this recent change: #5081

Have raised a seperate PR to address the above issue: #5102

k8s-ci-robot · 2025-09-03T04:40:13Z

Hi @mayuka-c. 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.

mayuka-c · 2025-09-08T05:54:49Z

Hi @camilamacedo86 Could you please review the PR whenever you have some time. Thanks :)
Also could you please run /ok-to-test if the changes are good

vitorfloriano · 2025-09-11T02:39:51Z

pkg/cli/alpha/internal/generate_test.go

+		oldPath         string
+		err             error
+	)
+	BeforeEach(func() {


Hi @mayuka-c I see there are some BeforeEach blocks that are identifical. Could we extract them to a helper function and then call that instead? I think this would reduce the code by dozens of lines and make the setup for each test group much cleaner and easier to read. WDYT?

Sure @vitorfloriano, will refactor things to a separate function. It does make sense to simplify and have cleaner test cases. Will be doing it, Thanks :)

Have done the refactoring, PTAL. Thanks!

After doing the rebase, had to make few updates. Please find the change done in description

vitorfloriano · 2025-09-16T00:05:45Z

pkg/cli/alpha/internal/generate.go

@mayuka-c Perhaps we should open a PR to address the issue you encountered in this file, with the motivation for the changes. That helps keep each PR within their own scope. This also helps in case we need to revert the changes in the PR.

You could then mark this PR (#5074) as blocked by the new PR.

Sounds good, I was thinking the same. But then as it is mainly done for mocking for helping with UT, decided to keep in the same PR. Will be raising a seperate PR for it. Thanks :)

Here's the seperate PR: #5102.

Have currently set this PR on hold till the above PR is merged and then will be rebasing the changes here so that the test cases will not fail. PTAL, Thanks :)

Now that we have the other PR we can revert the changes in this file.

I didnt want to revert it here as I didnt want the test cases to fail in this PR. Once my other PR is merged, will be rebasing the changes so that those changes dont appear here. I have made the current PR on hold till my other PR is merged. Hope this is fine

vitorfloriano · 2025-09-16T00:08:07Z

pkg/cli/alpha/internal/generate_test.go

+			It("should return correct args for plugins, domain, repo", func() {
+				cfg := &fakeConfig{pluginChain: []string{"go.kubebuilder.io/v3"}, domain: "foo.com", repo: "bar"}
+				store := &fakeStore{cfg: cfg}
+				args := getInitArgs(store)
+				Expect(args).To(ContainElements("--plugins", ContainSubstring("go.kubebuilder.io/v4"),
+					"--domain", "foo.com", "--repo", "bar"))
+			})
+			It("should return correct args for plugins, domain, repo", func() {


@mayuka-c These two tests have the same description. Could we improve that to make it more distinguishable?

Nice catch :). Have made the change to include a When block. PTAL

vitorfloriano · 2025-09-16T00:42:54Z

pkg/cli/alpha/internal/generate_test.go

+		Context("Without External flag set", func() {
+			It("runs kubebuilder create api successfully for a resource", func() {
+				res := resource.Resource{
+					GVK:        resource.GVK{Group: "example.com", Version: "v1", Kind: "Example", Domain: "external"},
+					Plural:     "examples",
+					API:        &resource.API{Namespaced: true},
+					Controller: true,
+					External:   true,


@mayuka-c This seems a bit confusing. The context says "Without External flag set", but we are setting External to true.

Looks like a typo from my end. Have updated the description. PTAL

mayuka-c · 2025-09-16T02:06:42Z

/hold
Waiting for the dependency PR to be merged: #5102

camilamacedo86 · 2025-09-24T13:31:40Z

pkg/cli/alpha/internal/generate_test.go

+			})
+			AfterEach(func() {
+				Expect(os.RemoveAll(tmpDir)).To(Succeed())
+				Expect(os.Setenv("PATH", oldPath)).To(Succeed())


If you look how the other tests are, you will see that we have a func in the lib to manage the dir and bin already see: https://github.com/kubernetes-sigs/kubebuilder/blob/master/pkg/cli/alpha/internal/common/common_test.go#L38-L44

Sure will be utilizing it, thanks

Hi @camilamacedo86
Sorry for my late reply, had been busy last week.
I have made the updates to utilize the func/utils. For few of the test cases, needed a mock kubebuilder, so I have added a func for that. Please suggest and let me know if its fine.

camilamacedo86

The tests are not using the funcs/utils that we use see for example: https://github.com/kubernetes-sigs/kubebuilder/blob/master/pkg/cli/alpha/internal/common/common_test.go#L38-L44

mayuka-c · 2025-10-16T12:06:16Z

/unhold

camilamacedo86 · 2025-11-02T09:22:09Z

/ok-to-test

lint fix Modify Tests Minor-1 Minor fix Minor fix 1 Minor fix 2 Minor fix 3 Minor fix 4 Fix - 4 Comments - 1 Use testContext func Fix UT after rebase Add Tests for change-dir

camilamacedo86

Hey @mayuka-c

Thank you a lot for your effort here 🎉

BUT: We already have some tests in place here:
👉 https://github.com/kubernetes-sigs/kubebuilder/blob/master/pkg/cli/alpha/generate_test.go

From what I can see, those tests seem to cover similar cases. Honestly, the issue was opened mainly to make sure we have coverage where it really makes sense — like CLI helpers and similar areas. But I think we already have quite a bit of good coverage.

If you find other places outside of the alpha commands that would benefit from additional tests, that would be great! I don’t think we need to aim for 100% coverage since our end-to-end tests are already very strong.

What do you think — do these tests make sense, or do they look duplicated to you?

mayuka-c · 2025-11-03T03:24:19Z

Hey @mayuka-c

Thank you a lot for your effort here 🎉

BUT: We already have some tests in place here: 👉 https://github.com/kubernetes-sigs/kubebuilder/blob/master/pkg/cli/alpha/generate_test.go

From what I can see, those tests seem to cover similar cases. Honestly, the issue was opened mainly to make sure we have coverage where it really makes sense — like CLI helpers and similar areas. But I think we already have quite a bit of good coverage.

If you find other places outside of the alpha commands that would benefit from additional tests, that would be great! I don’t think we need to aim for 100% coverage since our end-to-end tests are already very strong.

What do you think — do these tests make sense, or do they look duplicated to you?

Hi @camilamacedo86 👋
Yeah few of these are already been taken care above.

But these tests cover few test scenarios in detail like the migratePlugins (Helm, Grafana), utility func and also negative test cases for each func.

I'm fine with either way. Please feel free to close the PR if we feel this might not be required.
Thanks :)

camilamacedo86

Since it’s passing in CI, I think we can go ahead so we don’t lose the contribution 🥇
However, it might be a good idea to also check this file:

👉 https://github.com/kubernetes-sigs/kubebuilder/blob/master/pkg/cli/alpha/generate_test.go

Just to see if it would make sense to centralize all the tests there, and make sure we don’t have any duplicates, etc.

/lgtm

k8s-ci-robot · 2025-11-03T08:49:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, mayuka-c

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~~ [camilamacedo86]

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 the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 3, 2025

k8s-ci-robot requested a review from camilamacedo86 September 3, 2025 04:40

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 3, 2025

k8s-ci-robot requested a review from Kavinjsir September 3, 2025 04:40

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 3, 2025

mayuka-c mentioned this pull request Sep 3, 2025

🌱 (CLI) Add unit tests for cli alpha generate.go #5073

Closed

mayuka-c changed the title ~~Add unit tests for cli alpha generate.go~~ 🌱 (CLI) Add unit tests for cli alpha generate.go Sep 3, 2025

mayuka-c force-pushed the ut-alpha-generate branch 4 times, most recently from c6961d7 to 0ec9db7 Compare September 3, 2025 05:56

mayuka-c changed the title ~~🌱 (CLI) Add unit tests for cli alpha generate.go~~ 🌱 (CLI) Add unit tests for cli alpha generate.go Sep 4, 2025

vitorfloriano reviewed Sep 11, 2025

View reviewed changes

mayuka-c force-pushed the ut-alpha-generate branch 2 times, most recently from dac18fe to 7f381b0 Compare September 15, 2025 15:00

vitorfloriano reviewed Sep 16, 2025

View reviewed changes

mayuka-c force-pushed the ut-alpha-generate branch from 7f381b0 to 63071ff Compare September 16, 2025 01:52

mayuka-c mentioned this pull request Sep 16, 2025

🌱 (CLI - Alpha) Introduce Mockable getExecutablePath for Testing in Generate #5102

Closed

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 16, 2025

camilamacedo86 reviewed Sep 24, 2025

View reviewed changes

camilamacedo86 requested changes Sep 24, 2025

View reviewed changes

mayuka-c force-pushed the ut-alpha-generate branch 3 times, most recently from 2100507 to 87c3bcf Compare October 9, 2025 13:35

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 16, 2025

camilamacedo86 closed this Nov 2, 2025

camilamacedo86 reopened this Nov 2, 2025

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 Nov 2, 2025

Add unit tests for cli alpha generate.go

b76d072

lint fix Modify Tests Minor-1 Minor fix Minor fix 1 Minor fix 2 Minor fix 3 Minor fix 4 Fix - 4 Comments - 1 Use testContext func Fix UT after rebase Add Tests for change-dir

mayuka-c force-pushed the ut-alpha-generate branch from 87c3bcf to b76d072 Compare November 2, 2025 17:21

camilamacedo86 requested changes Nov 2, 2025

View reviewed changes

camilamacedo86 approved these changes Nov 3, 2025

View reviewed changes

k8s-ci-robot assigned camilamacedo86 Nov 3, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 3, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 3, 2025

k8s-ci-robot merged commit d792432 into kubernetes-sigs:master Nov 3, 2025
18 checks passed

🌱 (CLI) Add unit tests for cli alpha generate.go #5074

🌱 (CLI) Add unit tests for cli alpha generate.go #5074

Conversation

mayuka-c commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Sep 3, 2025

Uh oh!

mayuka-c commented Sep 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mayuka-c Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mayuka-c Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mayuka-c commented Sep 16, 2025

Uh oh!

camilamacedo86 Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 left a comment

Choose a reason for hiding this comment

Uh oh!

mayuka-c commented Oct 16, 2025

Uh oh!

camilamacedo86 commented Nov 2, 2025

Uh oh!

camilamacedo86 left a comment

Choose a reason for hiding this comment

Uh oh!

mayuka-c commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

camilamacedo86 left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Nov 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mayuka-c commented Sep 3, 2025 •

edited

Loading

mayuka-c Sep 16, 2025 •

edited

Loading

mayuka-c Sep 17, 2025 •

edited

Loading

camilamacedo86 Sep 24, 2025 •

edited

Loading

mayuka-c commented Nov 3, 2025 •

edited

Loading