update test fn-render/multiple-fnconfig #1839
Conversation
|
LGTM |
| @@ -13,4 +13,4 @@ | |||
| # limitations under the License. | |||
|
|
|||
| exitCode: 1 | |||
| skip: true # TODO: enable once the testcase is supported | |||
| stdErr: "following fields are mutually exclusive: 'config', 'configMap', 'configPath'. Got \"configPath, configMap\"" | |||
There was a problem hiding this comment.
We should address the following in a followup PR:
Improve the error message to fit our error-style ?
Wondering how to convey oneOf fields in more user friendly way: One option could be:
"function config must have only config or configMap or configPath input"
We also need the pkg and function involved in this error to make it really actionable. I have some ideas on introducing a new validationError, let have a chat offline about that.
There was a problem hiding this comment.
Yes, every message needs to meet go/kpt-errors-style-guide.
In this case, I see the following issues:
- We should tell users why they CAN do. They can specify only one of the 3 fields.
- Quotation issue:
"configPath, configMap"(The user is not providing a string with value ofConfigPath, configMap
There was a problem hiding this comment.
Added a comment about this to #1557; let me know if there should be a separate issue instead.
There was a problem hiding this comment.
For a specific error message I think we should create a separate issue otherwise #1557 will be longer and longer.
This test case passes so we don't need to skip it