-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: simplify go-test #301
Conversation
run: | | ||
OP_NS_OPT="" | ||
CLUSTER_NS_OPT="" | ||
test -n "${{ input.op-ns }}" && OP_NS_OPT="--operator-namespace ${{ input.op-ns }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test -n "${{ input.op-ns }}" && OP_NS_OPT="--operator-namespace ${{ input.op-ns }}" | |
test -n "${{ input.op-ns }}" && OP_NS_OPT="--operator-namespace ${{ input.op-ns }}" |
test -n
here is for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's old code. OP_NS_OPT should be set iff input.op-ns isn't rook-ceph. CLUSTER_NS_OPT should also be set iff input.cluster-ns isn't rook-ceph. I'll fix them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also CI is not running please check that also @satoru-takeuchi thanks
f8e8b7b
to
6e2b741
Compare
6e2b741
to
d8b362a
Compare
7e99e04
to
956e28a
Compare
2a40197
to
ca36048
Compare
2a841d0
to
5a3891c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we want one of the CI runs to run on a different namespace than rook-ceph
run: tests/github-action-helper.sh deploy_rook | ||
|
||
- name: deploy rook cluster in custom namespace | ||
shell: bash --noprofile --norc -eo pipefail -x {0} | ||
if: inputs.op-ns != '' || inputs.cluster-ns != '' | ||
if: inputs.op-ns != 'rook-ceph' || inputs.cluster-ns != 'rook-ceph' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if: inputs.op-ns != 'rook-ceph' || inputs.cluster-ns != 'rook-ceph' | |
if: inputs.op-ns != '=test-operator' || inputs.cluster-ns != '=test-cluster' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You meant if: inputs.op-ns == 'test-operator' || inputs.cluster-ns == 'test-cluster'
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, typo above
Remove duplicated description between the default configuration and the custom namespace configuration. Signed-off-by: Satoru Takeuchi <[email protected]>
5a3891c
to
36b237a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Remove duplicated description between the default configuration and the custom namespace configuration.
Checklist: