Skip to content

Conversation

marckhouzam
Copy link
Contributor

What this PR does / why we need it

There was a change in Cobra 1.9.0 which caused our unit tests to fail and showed that calling rootCmd.SetArgs(spec.args) is the proper approach when setting arguments to a cobra command in tests, even if os.Args is also used.

Also, because of the way we handle the help command for plugins in getHelpArguments(), we must also continue to set os.Args; however, the way the tests were written before this PR would not cause a failure if the call to os.Args was removed.

This commit updates the tests to better handle both cases.

Note that because of impacts to other programs, the particular change in Cobra was reverted in release v1.9.1.

Which issue(s) this PR fixes

Fixes # N/A

Describe testing done for PR

Only impacts tests, so CI.

Release note

Improve testing for plugins' help command

Additional information

Special notes for your reviewer

There was a change in Cobra 1.9.0 which caused our unit tests to fail
and showed that calling "rootCmd.SetArgs(spec.args)" is the proper
approach when setting arguments to a cobra command in tests.

Also, because of the way we handle the help command for plugins in
"getHelpArguments()", we must also continue to set "os.Args"; however,
the way the tests were written before would not cause a failure if
the call to "os.Args" was removed.

This commit updates the tests for both cases.

Signed-off-by: Marc Khouzam <[email protected]>
@marckhouzam marckhouzam requested a review from a team as a code owner February 25, 2025 14:04
Copy link
Contributor

@prkalle prkalle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the changes!

@marckhouzam marckhouzam merged commit 42ddaab into vmware-tanzu:main Feb 26, 2025
7 checks passed
@marckhouzam marckhouzam added this to the v1.5.4 milestone Feb 26, 2025
@marckhouzam marckhouzam deleted the marck/improveTest branch February 26, 2025 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants