Skip to content

Commit

Permalink
Silence stderr in tests (#1708)
Browse files Browse the repository at this point in the history
Resolves #1642: our test
output [is nice and tidy
again](https://app.circleci.com/pipelines/github/tiny-pilot/tinypilot/4286/workflows/253f8f8b-b3af-47f8-82dc-b88e22d1122b/jobs/29448?invite=true#step-103-8652_129).

Contrary to what I initially mentioned in the ticket, one of the two
stacktraces didn’t come from
[`test_process_with_result_child_exception`](https://github.com/tiny-pilot/tinypilot/blob/cd3f5ff30a04659a750aec6dbf60ca49927f10f0/app/execute_test.py#L58-L71),
which already had the “[stderr
silencing](https://github.com/tiny-pilot/tinypilot/blob/cd3f5ff30a04659a750aec6dbf60ca49927f10f0/app/execute_test.py#L61)”,
but rather from
[`test_background_thread_ignores_function_exception`](https://github.com/tiny-pilot/tinypilot/blob/cd3f5ff30a04659a750aec6dbf60ca49927f10f0/app/execute_test.py#L99-L100).
(I’ve updated the ticket accordingly.)

The “stderr silencing” technique is working fine, we just didn’t have it
on the other two functions. These were added later, so we likely just
forgot about this issue.

I’ve extracted a context manager function, to make it easier to discover
and re-use this technique in the tests, and to avoid us having to repeat
the explanatory comment + exact invocation again and again.
<a data-ca-tag
href="https://codeapprove.com/pr/tiny-pilot/tinypilot/1708"><img
src="https://codeapprove.com/external/github-tag-allbg.png" alt="Review
on CodeApprove" /></a>

---------

Co-authored-by: Jan Heuermann <[email protected]>
  • Loading branch information
jotaen4tinypilot and jotaen authored Dec 20, 2023
1 parent d74d358 commit 1dfa8cd
Showing 1 changed file with 20 additions and 4 deletions.
24 changes: 20 additions & 4 deletions app/execute_test.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import contextlib
import io
import time
import unittest
Expand All @@ -12,11 +13,16 @@
# processes[1], which pickles these functions[2]. So, they must be defined
# using `def` at the top level of a module[3].
#
# Another by-effect of this is that the `silence_stderr` context manager
# doesn’t have any effect on MacOS systems, so we cannot prevent the stacktrace
# printing there.[4]
#
# This was observed on a 2021 Macbook Pro M1 Max running OSX Ventura 13.2.1.
#
# [1] https://github.com/python/cpython/commit/17a5588740b3d126d546ad1a13bdac4e028e6d50
# [2] https://docs.python.org/3.9/library/multiprocessing.html#the-spawn-and-forkserver-start-methods
# [3] https://docs.python.org/3.9/library/pickle.html#what-can-be-pickled-and-unpickled:~:text=(using%20def%2C%20not%20lambda)
# [4] https://github.com/tiny-pilot/tinypilot/issues/1713


def do_nothing():
Expand All @@ -35,6 +41,14 @@ def return_string():
return 'Done!'


@contextlib.contextmanager
def silence_stderr():
"""Silences stderr to avoid polluting the terminal output of the tests."""
# Note: on MacOS systems, this doesn’t have an effect (see comment above).
with mock.patch('sys.stderr', io.StringIO()):
yield None


class ExecuteTest(unittest.TestCase):

def test_process_with_result_child_completed(self):
Expand All @@ -58,7 +72,7 @@ def test_process_with_result_child_not_completed(self):
def test_process_with_result_child_exception(self):
# Silence stderr while the child exception is being raised to avoid
# polluting the terminal output.
with mock.patch('sys.stderr', io.StringIO()):
with silence_stderr():
process = execute.ProcessWithResult(target=raise_exception,
daemon=True)
process.start()
Expand Down Expand Up @@ -89,12 +103,14 @@ def test_execute_with_timeout_return_value(self):
self.assertEqual('Done!', return_value)

def test_execute_with_timeout_child_exception(self):
with self.assertRaises(Exception) as ctx:
execute.with_timeout(raise_exception, timeout_in_seconds=0.5)
with silence_stderr():
with self.assertRaises(Exception) as ctx:
execute.with_timeout(raise_exception, timeout_in_seconds=0.5)
self.assertEqual('Child exception', str(ctx.exception))

def test_background_thread_ignores_function_successful(self):
self.assertEqual(None, execute.background_thread(return_string))

def test_background_thread_ignores_function_exception(self):
self.assertEqual(None, execute.background_thread(raise_exception))
with silence_stderr():
self.assertEqual(None, execute.background_thread(raise_exception))

0 comments on commit 1dfa8cd

Please sign in to comment.