fix(cmd): reorder invoke validation to check Initialized before Validate#3849
fix(cmd): reorder invoke validation to check Initialized before Validate#3849Elvand-Lie wants to merge 1 commit into
Conversation
|
Hi @Elvand-Lie. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3849 +/- ##
==========================================
+ Coverage 53.44% 53.47% +0.02%
==========================================
Files 200 200
Lines 23450 23431 -19
==========================================
- Hits 12533 12529 -4
+ Misses 9662 9654 -8
+ Partials 1255 1248 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/ok-to-test |
|
Other commands ( |
|
/assign |
1ac2196 to
2b1e9d6
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Elvand-Lie The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
2b1e9d6 to
5c026dc
Compare
|
@gauron99 pushed an update to match the repository's established error and validation patterns:
Let me know if my solution fits. |
There was a problem hiding this comment.
Pull request overview
This PR makes func invoke report the same not-initialized error style as other commands when run outside a function directory, instead of validating an uninitialized function first.
Changes:
- Checks
f.Initialized()beforef.Validate()inrunInvoke(). - Adds an invoke-specific
ErrNotInitializedhelp message. - Adds a unit test for invoking from a non-function directory.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
cmd/invoke.go |
Reorders initialization validation before function validation. |
cmd/errors.go |
Adds the invoke-specific not-initialized error message. |
cmd/invoke_test.go |
Covers the not-initialized invoke error path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Move !f.Initialized() check before f.Validate(), matching every other command (build, deploy, run, delete, describe) - Replace raw fmt.Errorf with NewErrNotInitializedFromPath(f.Root, invoke) for consistency - Remove fmt.Printf double error output on validation failure - Add invoke case to cmd/errors.go for standardized CLI error messaging - Add TestInvoke_NotInitialized to verify the correct error type and message
6c68f64 to
d73aa6a
Compare
Fixes #3848
Changes
In
runInvoke(),f.Validate()was called before checkingf.Initialized(). Every other command (run,deploy,delete,describe,subscribe) checksInitialized()immediately afterNewFunction(). Theinvokecommand was the only one that validated first.This caused users running
func invokein a non-function directory to see a confusing validation error (e.g.,func.yaml contains errors) instead of the helpful "no function found" message.Additionally, the
Validateerror was printed viafmt.PrintfAND returned to cobra, causing double error output.What this PR does:
!f.Initialized()check beforef.Validate()— matching every other commandfmt.Printfdouble error output on validation failureTesting
The reordering ensures the
!f.Initialized()guard runs first, returning the user-friendly error message. Thef.Validate()call only runs on initialized functions where it can produce meaningful diagnostics./kind bug