-
Notifications
You must be signed in to change notification settings - Fork 0
Fix minor issues in e2e-tests-latest-eventing.sh #1
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
base: upgrade-test
Are you sure you want to change the base?
Conversation
This PR adds the tests to verify the correct number and names of knative eventing deployments. The test tag postupgrade is added, marking the tests to run after upgrade to the latest HEAD of operator, with the latest generated manifest of knative eventing.
|
|
||
| OPERATOR_DIR=$(dirname $0)/.. | ||
| # inherited from vendor/knative.dev/test-infra/scripts/library.sh | ||
| declare -A IS_PROW |
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.
this is to be used later. it is inherited from the library.sh in test-infra
| # inherited from vendor/knative.dev/test-infra/scripts/library.sh | ||
| declare -A IS_PROW | ||
|
|
||
| OPERATOR_DIR=$(cd $(dirname "$0")/.. && pwd) |
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.
Changed
OPERATOR_DIR=$(dirname $0)/.. to
OPERATOR_DIR=$(cd $(dirname "$0")/.. && pwd)
.
First one results in a relative path and when we cd into eventing folder later in the script, cding back with that relative path goes to another directory relative to eventing, not eventing-operator.
Second one results in an absolute path and cding to that path always works.
| if (( IS_PROW )); then | ||
| ko resolve -P -f ${config}/ | "${LABEL_YAML_CMD[@]}" > ${yaml} | ||
| else | ||
| ko resolve -f ${config}/ | "${LABEL_YAML_CMD[@]}" > ${yaml} | ||
| fi |
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.
Don't use -P when the script is running local. Can't push to Docker when image paths are preserved.
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.
Well, I switched to Quay and this problem is gone.
388f348 to
43d7852
Compare
Some improvements. Comments inline