Skip to content

Commit

Permalink
Output stderr in V2 test rule (pantsbuild#7694)
Browse files Browse the repository at this point in the history
### Problem
Implements pantsbuild#7388. Swallowing stderr makes debugging very difficult.

Important clarification: failing tests will write to stdout, meaning that we do already output any message captured by Pytest. Instead, this PR is meant to log issues that occur before Pytest executes, such as issues from Pants.

### Solution
Follow the convention established by pantsbuild#7676 to output `{address} stderr:`, followed by the stderr.
  • Loading branch information
Eric-Arellano committed May 10, 2019
1 parent 55e5721 commit c0bfc9e
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 13 deletions.
7 changes: 5 additions & 2 deletions src/python/pants/backend/python/rules/python_test_runner.py
Expand Up @@ -106,10 +106,13 @@ def run_python_test(transitive_hydrated_target, pytest):
)

result = yield Get(FallibleExecuteProcessResult, ExecuteProcessRequest, request)
# TODO: Do something with stderr?
status = Status.SUCCESS if result.exit_code == 0 else Status.FAILURE

yield PyTestResult(status=status, stdout=result.stdout.decode('utf-8'))
yield PyTestResult(
status=status,
stdout=result.stdout.decode('utf-8'),
stderr=result.stderr.decode('utf-8'),
)


def rules():
Expand Down
3 changes: 2 additions & 1 deletion src/python/pants/rules/core/core_test_model.py
Expand Up @@ -15,7 +15,8 @@ class Status(enum(['SUCCESS', 'FAILURE'])): pass
class TestResult(datatype([
('status', Status),
# The stdout of the test runner (which may or may not include actual testcase output).
('stdout', text_type)
('stdout', text_type),
('stderr', text_type),
])):

# Prevent this class from being detected by pytest as a test class.
Expand Down
15 changes: 12 additions & 3 deletions src/python/pants/rules/core/test.py
Expand Up @@ -27,15 +27,24 @@ def fast_test(console, addresses):
test_results = yield [Get(TestResult, Address, address.to_address()) for address in addresses]
did_any_fail = False
for address, test_result in zip(addresses, test_results):
if test_result.status == Status.FAILURE:
did_any_fail = True
if test_result.stdout:
console.write_stdout(
"{} stdout:\n{}\n".format(
address.reference(),
console.red(test_result.stdout) if test_result.status == Status.FAILURE else test_result.stdout
)
)
if test_result.status == Status.FAILURE:
did_any_fail = True
if test_result.stderr:
# NB: we write to stdout, rather than to stderr, to avoid potential issues interleaving the
# two streams.
console.write_stdout(
"{} stderr:\n{}\n".format(
address.reference(),
console.red(test_result.stderr) if test_result.status == Status.FAILURE else test_result.stderr
)
)

console.write_stdout("\n")

Expand All @@ -58,7 +67,7 @@ def coordinator_of_tests(target):
# See https://github.com/pantsbuild/pants/issues/4535
if target.adaptor.type_alias == 'python_tests':
result = yield Get(PyTestResult, HydratedTarget, target)
yield TestResult(status=result.status, stdout=result.stdout)
yield TestResult(status=result.status, stdout=result.stdout, stderr=result.stderr)
else:
raise Exception("Didn't know how to run tests for type {}".format(target.adaptor.type_alias))

Expand Down
26 changes: 19 additions & 7 deletions tests/python/pants_test/rules/test_test.py
Expand Up @@ -39,7 +39,7 @@ def make_build_target_address(self, spec):

def test_outputs_success(self):
self.single_target_test(
result=TestResult(status=Status.SUCCESS, stdout='Here is some output from a test'),
result=TestResult(status=Status.SUCCESS, stdout='Here is some output from a test', stderr=''),
expected_console_output=dedent("""\
some/target stdout:
Here is some output from a test
Expand All @@ -50,7 +50,7 @@ def test_outputs_success(self):

def test_output_failure(self):
self.single_target_test(
result=TestResult(status=Status.FAILURE, stdout='Here is some output from a test'),
result=TestResult(status=Status.FAILURE, stdout='Here is some output from a test', stderr=''),
expected_console_output=dedent("""\
some/target stdout:
Here is some output from a test
Expand All @@ -67,9 +67,9 @@ def test_output_mixed(self):

def make_result(target):
if target == target1:
return TestResult(status=Status.SUCCESS, stdout='I passed\n')
return TestResult(status=Status.SUCCESS, stdout='I passed\n', stderr='')
elif target == target2:
return TestResult(status=Status.FAILURE, stdout='I failed\n')
return TestResult(status=Status.FAILURE, stdout='I failed\n', stderr='')
else:
raise Exception("Unrecognised target")

Expand All @@ -90,21 +90,33 @@ def make_result(target):
testprojects/tests/python/pants/fails ..... FAILURE
"""))

def test_stderr(self):
self.single_target_test(
result=TestResult(status=Status.FAILURE, stdout='', stderr='Failure running the tests!'),
expected_console_output=dedent("""\
some/target stderr:
Failure running the tests!
some/target ..... FAILURE
"""),
success=False,
)

def test_coordinator_python_test(self):
target_adaptor = PythonTestsAdaptor(type_alias='python_tests')

result = run_rule(coordinator_of_tests, HydratedTarget(Address.parse("some/target"), target_adaptor, ()), {
(PyTestResult, HydratedTarget): lambda _: PyTestResult(status=Status.FAILURE, stdout='foo'),
(PyTestResult, HydratedTarget): lambda _: PyTestResult(status=Status.FAILURE, stdout='foo', stderr=''),
})

self.assertEqual(result, TestResult(status=Status.FAILURE, stdout='foo'))
self.assertEqual(result, TestResult(status=Status.FAILURE, stdout='foo', stderr=''))

def test_coordinator_unknown_test(self):
target_adaptor = PythonTestsAdaptor(type_alias='unknown_tests')

with self.assertRaises(Exception) as cm:
run_rule(coordinator_of_tests, HydratedTarget(Address.parse("some/target"), target_adaptor, ()), {
(PyTestResult, HydratedTarget): lambda _: PyTestResult(status=Status.FAILURE, stdout='foo'),
(PyTestResult, HydratedTarget): lambda _: PyTestResult(status=Status.FAILURE, stdout='foo', stderr=''),
})

self.assertEqual(str(cm.exception), "Didn't know how to run tests for type unknown_tests")

0 comments on commit c0bfc9e

Please sign in to comment.