tolerate multiple =s in a parameter value #10880
Conversation
bparees
force-pushed
the
fix_process_parames
branch
from
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. |
bparees
force-pushed
the
fix_process_parames
branch
from
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] |
| # we should have overwritten the template param | ||
| os::cmd::expect_success_and_text 'oc get user someval -o jsonpath={.Name}' 'someval' | ||
| # providing a value not in the template should fail | ||
| 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.