-
Notifications
You must be signed in to change notification settings - Fork 859
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test/e2e/karmadactl_test.go: test version command #5192
test/e2e/karmadactl_test.go: test version command #5192
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #5192 +/- ##
=======================================
Coverage 28.25% 28.25%
=======================================
Files 632 632
Lines 43723 43723
=======================================
+ Hits 12354 12355 +1
+ Misses 30467 30466 -1
Partials 902 902
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
78ec6d3
to
c9b583e
Compare
test/e2e/karmadactl_test.go
Outdated
gomega.Expect(strings.Contains(err.Error(), "unknown flag: --invalidflag")).Should(gomega.BeTrue()) | ||
}) | ||
|
||
ginkgo.It("should return the version information with kubeconfig", func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mohamedawnallah Is this test case necessary? karmadactl version
doesn't actually use flag --kubeconfig
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right. When I started using the karmadactl options
command, I found the flag --kubeconfig
. However, you're correct that this is for testing something else and not the version
command. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will go ahead an remove it now!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks updated it!
c9b583e
to
c612a44
Compare
/lgtm |
hi, thanks, looks good to me |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Vacant2333, XiShanYongYe-Chang 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:
Approvers can indicate their approval by writing |
/hold |
Hi @mohamedawnallah there is a merge conflict, can you help rebase the master branch and push it again? |
In this commit, we test `karmadactl version` command to make sure it return valid information and return error when invalid flag is provided. Signed-off-by: Mohamed Awnallah <mohamedmohey2352@gmail.com>
c612a44
to
1fc977e
Compare
Rebased once the GitHub CI tests finish. I will ping you :) |
Thanks |
Description
In this commit, we test
karmadactl version
command to make sure it return valid information and return error when invalid flag is provided.What type of PR is this?
/kind failing-test
Which issue(s) this PR fixes:
Part of #4544
Does this PR introduce a user-facing change?: