tolerate multiple =s in a parameter value#10880
tolerate multiple =s in a parameter value#10880openshift-bot merged 1 commit intoopenshift:masterfrom
Conversation
ce0e2e9 to
796eac4
Compare
|
[test] |
|
@bparees can we add a test case to |
|
It's a regression but probably missed the train unless we respin. I'll pull it to 1.3.0 origin though. |
|
Add the test case and I'll review and merge. |
796eac4 to
4b0223e
Compare
|
@smarterclayton @stevekuznetsov TC added. Note I've also now fixed an issue where if you supplied a value of the form "--value=key", we panicked because we'd try to reference the second element of the split result, even if the split hadn't returned 2+ values. |
|
Evaluated for origin test up to 4b0223e |
|
LGTM [merge] |
| os::cmd::expect_failure_and_text "oc process -f '${required_params}' --value=required_param=someval --value=other_param=otherval" 'unknown parameter name "other_param"' | ||
| # failure on values fails the entire call | ||
| os::cmd::expect_failure_and_text "oc process -f '${required_params}' --value=required_param=someval --value=optional_param=some=series=of=values=" 'invalid parameter assignment in' | ||
| os::cmd::expect_failure_and_text "oc process -f '${required_params}' --value=required_param=someval --value=optional_param" 'invalid parameter assignment in' |
There was a problem hiding this comment.
What is this test doing now?
There was a problem hiding this comment.
same thing it did before. ensuring that if you supply an invalid parameter arg, the whole call fails w/ the correct error message. it's just that now it's incorrect because there's no equal sign as part of the arg, instead of before when it was incorrect because there were multiple equals in the arg.
|
Evaluated for origin merge up to 4b0223e |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8817/) (Image: devenv-rhel7_5003) |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8816/) |
@stevekuznetsov @smarterclayton this was introduced in 6eaf7a2 (thanks @fabianofranz for digging it up).
it seems like a fairly bad regression (https://bugzilla.redhat.com/show_bug.cgi?id=1375275)
so if we're still doing merges to 3.3, i'd say this is a candidate.