Skip to content
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

Make do_tests print out the error message in case a test fails #515

Merged
merged 4 commits into from Oct 24, 2016

Conversation

@jougs
Copy link
Contributor

jougs commented Oct 7, 2016

Debugging failing tests on Travis is a pain. This PR modifies do_tests.sh to print the script's output to the commandline in case an error occurs. I suggest @heplesser and @DimitriPlotnikov as reviewers.

Copy link
Contributor

heplesser left a comment

@jougs Thank you, this is an excellent idea! Before approving, I would still like to see that it works in practice. Could you temporarily add a failing test to demonstrate this?

if test "${msg_dirty}" != "${param_failure}" ; then
explanation="${msg_clean}"
else
explanation="Failed: unexpected exit code ${exit_code}"
msg_error="$( cat "${TEST_OUTFILE}" )"
unexpected_exitcode=true

This comment has been minimized.

Copy link
@heplesser

heplesser Oct 8, 2016

Contributor

Looks as if indentation is off by one.

This comment has been minimized.

Copy link
@jougs

jougs Oct 17, 2016

Author Contributor

Thanks for noticing. I used tabs instead of spaces. Fixed by ab14b92.

@heplesser
Copy link
Contributor

heplesser commented Oct 17, 2016

@jougs It works! Once you remove the intentionally failing test again, I will approve.

Copy link
Collaborator

gtrensch left a comment

👍 works nicely !

@DimitriPlotnikov
Copy link

DimitriPlotnikov commented Oct 19, 2016

@jougs I have one clarification question: what is precisely the added value of the script compared to 'make installcheck'?

@jougs
Copy link
Contributor Author

jougs commented Oct 19, 2016

@DimitriPlotnikov: It's not the case that one provides a benefit over the other. The two are parts of the same thing. do_tests.sh is run by the installcheck rule in the make process to run all tests in a sensible order.

@DimitriPlotnikov
Copy link

DimitriPlotnikov commented Oct 19, 2016

@jougs OK:)
Yes, also +1 from my side.

@jougs jougs dismissed heplesser’s stale review Oct 19, 2016

I've reverted the addition of the failing test.

@heplesser heplesser merged commit 65a1aba into nest:master Oct 24, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.