-
Notifications
You must be signed in to change notification settings - Fork 67
Checking e2e build succeeded #89
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -37,9 +37,15 @@ function run_buildpack_test() { | |||||
kubectl apply -f test/build-buildpack.yaml || return 1 | ||||||
# Wait 5s for processing to start | ||||||
sleep 5 | ||||||
echo "Checking that build was started:" | ||||||
kubectl get build buildpack-build -oyaml | ||||||
# TODO(adrcunha): Add proper verification. | ||||||
echo "Checking that build was successed:" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
for i in {1..100};do | ||||||
kubectl get build buildpack-build -o 'jsonpath={.status.conditions[?(@.type=="Succeeded")].status}' 1>/dev/null 2>&1 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume Jason agrees with this check? |
||||||
if [ $? -ne 1 ]; then | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. -eq 0 |
||||||
break | ||||||
fi | ||||||
echo "The build was successed." | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
sleep 2 | ||||||
done | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add some progress indicator here, I'm not sure how long the operation will take.
Prior art: https://github.com/knative/test-infra/blob/master/scripts/library.sh#L176 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't you fail the test if the build didn't succeed? Otherwise, this check adds no value. |
||||||
} | ||||||
|
||||||
# Script entry point. | ||||||
|
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.
I believe that you can remove this sleep now, since you're checking for a successful build.