Skip to content
This repository has been archived by the owner on Sep 6, 2019. It is now read-only.

Checking e2e build succeeded #89

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Checking e2e build succeeded #89

wants to merge 1 commit into from

Conversation

zouyee
Copy link
Contributor

@zouyee zouyee commented Feb 1, 2019

Checking e2e build succeeded

@knative-prow-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zouyee
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: srinivashegde86

If they are not already assigned, you can assign the PR to them by writing /assign @srinivashegde86 in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@zouyee
Copy link
Contributor Author

zouyee commented Feb 4, 2019

/cc @vdemeester

Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ok-to-test

fi
echo "waiting for build was successed."
sleep 2
done
# TODO(adrcunha): Add proper verification.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would need to be removed 👼

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping!

if [ $? -ne 1 ]; then
break
fi
echo "waiting for build was successed."

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about "waiting for build to be successful"?

@imjasonh
Copy link
Member

imjasonh commented Mar 6, 2019

cc @adrcunha as resident bash expert and original TODO author

echo "Checking that build was successed:"
for i in {1..100};do
kubectl get build buildpack-build -o 'jsonpath={.status.conditions[?(@.type=="Succeeded")].status}' 1>/dev/null 2>&1
if [ $? -ne 1 ]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-eq 0

fi
echo "The build was successed."
sleep 2
done
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

echo -n "."

Prior art:

https://github.com/knative/test-infra/blob/master/scripts/library.sh#L176

fi
echo "The build was successed."
sleep 2
done
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

# TODO(adrcunha): Add proper verification.
echo "Checking that build was successed:"
for i in {1..100};do
kubectl get build buildpack-build -o 'jsonpath={.status.conditions[?(@.type=="Succeeded")].status}' 1>/dev/null 2>&1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume Jason agrees with this check?

echo "Checking that build was started:"
kubectl get build buildpack-build -oyaml
# TODO(adrcunha): Add proper verification.
echo "Checking that build was successed:"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
echo "Checking that build was successed:"
echo -n "Waiting for build to finish"

@@ -37,9 +37,15 @@ function run_buildpack_test() {
kubectl apply -f test/build-buildpack.yaml || return 1
# Wait 5s for processing to start
Copy link
Contributor

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.

if [ $? -ne 1 ]; then
break
fi
echo "The build was successed."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
echo "The build was successed."
echo -e "\nBuild succeeded."

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants