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

Print console output for pytest to give context for timeouts #118

Closed
wants to merge 1 commit into from

Conversation

dhood
Copy link
Contributor

@dhood dhood commented Nov 22, 2017

Otherwise tests that timeout (as opposed to failing) are killed when they time out and their output is not shown in reports

This is the pytest equivalent of #98

Timeout without this PR: http://ci.ros2.org/view/nightly/job/nightly_linux_repeated/841/testReport/junit/(root)/projectroot/test_composition__rmw_fastrtps_cpp/

Timeout with this PR:
http://ci.ros2.org/job/ci_windows/3678/testReport/junit/(root)/projectroot/test_composition__rmw_fastrtps_cpp/ (you can see the output)

@dhood dhood added the in review Waiting for review (Kanban column) label Nov 22, 2017
@dhood dhood self-assigned this Nov 22, 2017
Copy link
Contributor

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

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

I think the reason for removing this is that the junit files end up empty if we don't capture:
ament/ament_tools@a3a9584.

Before this PR all the non-timed out jobs had content in the result file: http://ci.ros2.org/view/nightly/job/nightly_linux_debug/655/testReport/composition.build.composition/test_composition__rmw_fastrtps_cpp_Debug/test_api_pubsub_composition/

With this PR they end up empty: http://ci.ros2.org/job/ci_windows/3678/testReport/composition.build.composition/test_composition__rmw_connext_cpp_Release/test_api_pubsub_composition/

So I think we need to find another way (dont know which one) to get content in the result files and console output if timeout

@dhood
Copy link
Contributor Author

dhood commented Nov 22, 2017

Hmm ok, you're right.
I'll have to see if there's a way to get the best of both worlds (probably by not having cmake kill them when they timeout).

If we desperately want this, then the passing tests do have their output captured: http://ci.ros2.org/job/ci_windows/3678/testReport/(root)/projectroot/test_composition__rmw_connext_cpp/
but it's the output of how the test was run as opposed to the generated file for each test, so not ideal.

@mikaelarguedas
Copy link
Contributor

If we desperately want this, then the passing tests do have their output captured: http://ci.ros2.org/job/ci_windows/3678/testReport/(root)/projectroot/test_composition__rmw_connext_cpp/
but it's the output of how the test was run as opposed to the generated file for each test, so not ideal.

I was more concerned about failing tests (before timeout) rather than passing tests (even if it's always good to have result files for all of them). I couldnt find a test failing without timeout in the job you posted that's why I picked a passing one. Do you know if the result files are empty for failing tests as well?

I think it could be fine for jenkins display as each test output can be accessed fairly easily. For the case of running local tests It'll be more of a pain as the result files will be empty and people will need to browse their console output to figure out why a specific test fail without having to rerun only that test.

We could conclude (for now) that having console output for all tests in Jenkins outweigh the lack of content in the result files.

I'll place this in progress for now

@mikaelarguedas mikaelarguedas added in progress Actively being worked on (Kanban column) and removed in review Waiting for review (Kanban column) labels Nov 22, 2017
@dhood dhood force-pushed the pytest_no_capture branch from 6234dee to fc98a49 Compare March 9, 2018 22:42
@dhood
Copy link
Contributor Author

dhood commented Apr 13, 2018

FYI the --show-capture option added to pytest that @dirk-thomas mentioned applies to failed tests. Unfortunately it doesn't help with this issue because we don't have pytest failures but timeouts. Think we'd need something more like this pytest feature to get the best of both worlds.

What if we add the pytest-timeout pip package so pytest doesn't get SIGTERMed by ctest? https://pypi.python.org/pypi/pytest-timeout

Otherwise tests are killed when they time out (instead of fail) and their output is not shown in reports
@dhood dhood force-pushed the pytest_no_capture branch from fc98a49 to 7b0b2d1 Compare April 19, 2018 19:26
@dhood dhood added ready Work is about to start (Kanban column) and removed in progress Actively being worked on (Kanban column) labels May 10, 2018
@dirk-thomas
Copy link
Contributor

Closing due to no activity. Also just skipping capturing isn't the right solution anyway.

@dirk-thomas dirk-thomas closed this Feb 3, 2020
@dirk-thomas dirk-thomas deleted the pytest_no_capture branch February 3, 2020 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready Work is about to start (Kanban column)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants