🌱 (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:11Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mayuka-c
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 · 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

lint fix Modify Tests Minor-1 Minor fix Minor fix 1 Minor fix 2 Minor fix 3 Minor fix 4 Fix - 4 Comments - 1

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

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

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

Add unit tests for cli alpha generate.go

63071ff

lint fix Modify Tests Minor-1 Minor fix Minor fix 1 Minor fix 2 Minor fix 3 Minor fix 4 Fix - 4 Comments - 1

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

Open

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

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

Are you sure you want to change the base?

🌱 (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!

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!

Uh oh!

mayuka-c commented Sep 3, 2025 •

edited

Loading

mayuka-c Sep 16, 2025 •

edited

Loading

mayuka-c Sep 17, 2025 •

edited

Loading