fix annotations test flake #11443
fix annotations test flake #11443
Conversation
| @@ -13,16 +13,16 @@ trap os::test::junit::reconcile_output EXIT | |||
| os::test::junit::declare_suite_start "cmd/annotate" | |||
| # This test validates empty values in key-value pairs set by the annotate command | |||
| os::cmd::expect_success_and_text 'oc process -f examples/zookeeper/template.json | oc apply -f -' 'pod "zookeeper-1" created' | |||
| os::cmd::expect_success_and_text 'oc annotate pod zookeeper-1 node-selector=""' 'pod "zookeeper-1" annotated' | |||
| os::cmd::expect_success_and_text 'oc get pod zookeeper-1 --template="{{.metadata.annotations}}"' 'node-selector:' | |||
| os::cmd::expect_success_and_text 'oc annotate pod zookeeper-1 nodeselector=""' 'pod "zookeeper-1" annotated' | |||
There was a problem hiding this comment.
This change seems unrelated?
There was a problem hiding this comment.
It is, but it was not testing an empty value annotation well enough before. The line below it actually checks for the annotation's value now, as opposed to just the annotation name and a colon
There was a problem hiding this comment.
Right, but you can do that without renaming the annotation
There was a problem hiding this comment.
hm, I was getting error: error parsing template {{.metadata.annotations.node-selector}}, template: output:1: bad character U+002D '-' when I gave it the template {{.metadata.annotations.node-selector}}
There was a problem hiding this comment.
I can't remember my jsonpath well -- is there a object['field-name'] accessor?
There was a problem hiding this comment.
hm, apparently I can use '{{index .metadata.annotations "my-index"}}' whenever a key includes "bad characters"
There was a problem hiding this comment.
Sounds good a2057c2
|
|
||
| echo "annotate: ok" | ||
| os::test::junit::declare_suite_end | ||
|
|
||
| os::test::junit::declare_suite_start "cmd/label" | ||
| # This test validates empty values in key-value pairs set by the label command | ||
| os::cmd::expect_success_and_text 'oc label pod zookeeper-1 label2=""' 'pod "zookeeper-1" labeled' | ||
| os::cmd::expect_success_and_text 'oc get pod zookeeper-1 --template="{{.metadata.labels}}"' 'label2\: ' | ||
| os::cmd::expect_success_and_text 'oc get pod zookeeper-1 --template="{{.metadata.labels.label2}}"' '' |
There was a problem hiding this comment.
It is not obvious to me that this will test what you want it to ... I think we need to be explicit in the expected text:
os::cmd::expect_success_and_text 'oc get pod zookeeper-1 --template="{{.metadata.labels.label2}}"' '^$'
There was a problem hiding this comment.
Sounds good, will update
|
|
||
| echo "annotate: ok" | ||
| os::test::junit::declare_suite_end | ||
|
|
||
| os::test::junit::declare_suite_start "cmd/label" | ||
| # This test validates empty values in key-value pairs set by the label command | ||
| os::cmd::expect_success_and_text 'oc label pod zookeeper-1 label2=""' 'pod "zookeeper-1" labeled' | ||
| os::cmd::expect_success_and_text 'oc get pod zookeeper-1 --template="{{.metadata.labels}}"' 'label2\: ' | ||
| os::cmd::expect_success_and_text 'oc get pod zookeeper-1 --template="{{.metadata.labels.label2}}"' '' |
There was a problem hiding this comment.
if you want to ensure the empty value, you need to pin to start and end: '^$'
if the label is completely missing, you'll get '' from the template output and '' will match that since it does substring regex match
| os::cmd::expect_success_and_text 'oc annotate pod zookeeper-1 node-selector=""' 'pod "zookeeper-1" annotated' | ||
| os::cmd::expect_success_and_text 'oc get pod zookeeper-1 --template="{{.metadata.annotations}}"' 'node-selector:' | ||
| os::cmd::expect_success_and_text 'oc annotate pod zookeeper-1 nodeselector=""' 'pod "zookeeper-1" annotated' | ||
| os::cmd::expect_success_and_text 'oc get pod zookeeper-1 --template="{{.metadata.annotations.nodeselector}}"' '' |
There was a problem hiding this comment.
same comment here, if you want to test empty value, pin '^$'
|
@stevekuznetsov @liggitt Thanks for the feedback, expecting |
|
@stevekuznetsov or @liggitt do you have any more feedback? Using |
juanvallejo
force-pushed
the
jvallejo/fix-annotations-test-flake
branch
from
dde5cda to
a2057c2
Compare
|
Squash, LGTM |
juanvallejo
force-pushed
the
jvallejo/fix-annotations-test-flake
branch
from
a2057c2 to
68cd54a
Compare
Done! |
|
[merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10424/) (Image: devenv-rhel7_5216) |
|
Evaluated for origin merge up to 68cd54a |
|
[Test]ing while waiting on the merge queue |
|
Evaluated for origin test up to 68cd54a |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10424/) (Base Commit: 2dccff1) |
Fixes #11442
@danwinship @openshift/cli-review