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.

Sign up for GitHub

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

Jump to bottom

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
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f608ea3
Add a regression test for scalative/haas#182
pzahemszky Nov 3, 2019
19fd79b
Use CLI buffer setting for the parallel runner too
pzahemszky Nov 3, 2019
5c7726b
Remove a comment
pzahemszky Nov 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions haas/plugins/parallel_runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ def __call__(self, result):
self.results.append(result)


def _run_test_in_process(test_case):
def _run_test_in_process(test_case, buffer):
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.

result_collector = ResultCollector(buffer=buffer)
result_collector.add_result_handler(result_handler)
runner = BaseTestRunner()
runner.run(result_collector, test_case)
Expand Down Expand Up @@ -131,12 +131,13 @@ def callback(collected_result):
error_tests.append(test_case)
else:
call_result = pool.apply_async(
_run_test_in_process, args=(test_case,),
_run_test_in_process, args=(test_case, result.buffer),
callback=callback)
call_results.append(call_result)

for test_case in error_tests:
collected_result = _run_test_in_process(test_case)
collected_result = _run_test_in_process(
test_case, result.buffer)
callback(collected_result)
finally:
pool.close()
Expand Down
33 changes: 33 additions & 0 deletions haas/tests/test_parallel_runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
from __future__ import print_function

from argparse import ArgumentParser
from datetime import datetime, timedelta
import sys
import time

from mock import Mock, patch
Expand All @@ -25,6 +28,7 @@ def wait(self, timeout):


def apply_async(func, args=None, kwargs=None, callback=None):
""" Run the given function serially (rather than in parallel) """
if args is None:
args = ()
if kwargs is None:
Expand Down Expand Up @@ -277,6 +281,35 @@ def test_parallel_runner_constructor_initializer(self, pool_class):
pool.close.assert_called_once_with()
pool.join.assert_called_once_with()

@patch('haas.plugins.parallel_runner.Pool')
def test_parallel_runner_stdout(self, pool_class):
# Given
pool = Mock()
pool_class.return_value = pool
pool.apply_async.side_effect = apply_async

def test_method():
print("STDOUT side effect of `test_method()`")
print("STDERR side effect", file=sys.stderr)
test_case = _test_cases.TestCase('test_method')
test_case.test_method = test_method
test_suite = TestSuite([test_case])

result_collector = ResultCollector(buffer=False)
runner = ParallelTestRunner()

patch_stdout = patch('sys.stdout', new=StringIO())
patch_stderr = patch('sys.stderr', new=StringIO())

# When
with patch_stdout as mock_stdout, patch_stderr as mock_stderr:
runner.run(result_collector, test_suite)

# Then
self.assertEqual(
mock_stdout.getvalue(), "STDOUT side effect of `test_method()`\n")
self.assertEqual(mock_stderr.getvalue(), "STDERR side effect\n")


class TestParallelRunnerImportError(unittest.TestCase):

Expand Down