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

Expose the --buffer setting for the parallel test runner too #183

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pzahemszky
Copy link

This pull request makes the command-line --buffer setting take effect for the parallel test runner too. Since it's off by default, the parallel runner will display the contents of the standard output (and those of the standard error) unless asked not to. Fixes #182.


Using the test_haas_parallel.py example script from ticket #182:

$ haas --runner parallel test_haas_parallel.py
one
two
..
----------------------------------------------------------------------
Ran 2 tests in 0.132s

OK
$ haas --runner parallel --buffer test_haas_parallel.py
..
----------------------------------------------------------------------
Ran 2 tests in 0.121s

OK

result_handler = ChildResultHandler()
result_collector = ResultCollector(buffer=True)
Copy link
Author

Choose a reason for hiding this comment

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

Note, that I don't fully understand the usage of ResultCollector, and in particular why its instantiation happens inside _run_test_in_process(). This is a simple attempt at solving issue #182.

Copy link
Contributor

Choose a reason for hiding this comment

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

The currentl implementation is very similar to setting buffer=False which results in output from the test going to the parent sys.stdout and sys.stderr. So the rest of the changes are not necessary.

Yet, not buffering the test output is not really a correct solution to the issue #182. The main problem is that test executed in parallel will write into the parent stdout/stderr streams at the same time making the results unreadable.

The correct solution is to buffer the output for each test and print it out in the parent process when the result has been collected. This way the output for each test will be selfcontained and readable.

Unfortunately, at the moment while both buffer stdout and stderr streams are created (i.e.buffer=True). When the results are accumulated, the stdout/stderr content is ignored unless there was a test exception/failure (see https://github.com/scalative/haas/blob/master/haas/result.py#L249). I think that fixing #182 will invlove fixing the behaviour (and maybe design) of the TestResult class.

@itziakos
Copy link
Contributor

itziakos commented Nov 7, 2019

Thanks @pzahemszky. I will have a look over the weekend.

@pzahemszky
Copy link
Author

Thank you, Ioannis.

Do you happen to know if the Python 2.6 and 3.3 jobs can be removed? They are failing on master too. If so, I'm happy to open a separate pull request for changing .travis.yml.

@itziakos
Copy link
Contributor

Thank you, Ioannis.
Do you happen to know if the Python 2.6 and 3.3 jobs can be removed? They are failing on master too. If so, I'm happy to open a separate pull request for changing .travis.yml.

The python 2.6 and 3.3 failures are not related to this PR and we need to investigate before taking any action.

Copy link
Contributor

@itziakos itziakos left a comment

Choose a reason for hiding this comment

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

The current implementation is a limited fix of the bug, so I cannot recommend that it is merged.

result_handler = ChildResultHandler()
result_collector = ResultCollector(buffer=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

The currentl implementation is very similar to setting buffer=False which results in output from the test going to the parent sys.stdout and sys.stderr. So the rest of the changes are not necessary.

Yet, not buffering the test output is not really a correct solution to the issue #182. The main problem is that test executed in parallel will write into the parent stdout/stderr streams at the same time making the results unreadable.

The correct solution is to buffer the output for each test and print it out in the parent process when the result has been collected. This way the output for each test will be selfcontained and readable.

Unfortunately, at the moment while both buffer stdout and stderr streams are created (i.e.buffer=True). When the results are accumulated, the stdout/stderr content is ignored unless there was a test exception/failure (see https://github.com/scalative/haas/blob/master/haas/result.py#L249). I think that fixing #182 will invlove fixing the behaviour (and maybe design) of the TestResult class.

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

Successfully merging this pull request may close these issues.

Parallel test runner swallows standard output
2 participants