-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add e2e test #16
Add e2e test #16
Conversation
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.
How about removing travis.yml?
test/e2e_test.sh
Outdated
--forward "http://127.0.0.1:$SERVER_PORT" 2>&1 | tag "[SERVER]" & | ||
|
||
function finish { | ||
kill $(jobs -p) || true |
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.
Why is || true
needed?
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.
Processes may die between jobs
and kill
. kill
will complain that 'there is no such process', and because of set -e
, the script halts its execution there. Should I set +e
on the exit trap?
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'm confused about the behavior of set -e
. Doesn't it make e2e_test.sh
stop when jobs
exits with non-zero?
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 it does. set -e
turns on the behavior, and set +e
turns off it.
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, if it does, why do you need || true
? The script will terminate when jobs exit with a non-zero value.
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 wanted to make sure wait
is called. It waits for remaining child processes to clean up and reports whether they are terminated properly.
f389fb2
to
9f0ee62
Compare
9f0ee62
to
f149bfa
Compare
Test fails, but it's a problem that needs to be addressed in other PRs.