From 7d69fccda74779cb3dcba2a7813c373de1321b2b Mon Sep 17 00:00:00 2001 From: jonathan keinan Date: Sun, 27 Sep 2026 14:37:49 +0300 Subject: [PATCH] Skip tearDown when setUp fails, and never let tearDown crash the worker _runTest called after_func() (tearDown) unconditionally in a bare finally, even when before_func() (setUp) raised. A tearDown written against a successful setUp -- e.g. one that reads self.env -- then raises its own exception (typically AttributeError, since the attribute setUp would have assigned was never set). That second exception escaped _runTest uncaught: unlike the original setUp/test failure, it bypasses handleFailure entirely. In the parallel coordinator this kills the whole worker process instead of failing just the one test. Because RLTest schedules different methods of the same test class across different workers, a single flaky setUp (e.g. a transient env-startup connection refusal) can take down every worker in the pool near-simultaneously, and the coordinator has no way to tell a genuine "no more processors are alive" condition apart from one that will resolve once a worker finishes flushing its queued output. Track whether setUp completed, skip tearDown if it didn't, and route any exception tearDown itself raises through handleFailure like any other test failure instead of letting it propagate. --- RLTest/__main__.py | 20 ++++- .../unit/test_teardown_after_failed_setup.py | 79 +++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 tests/unit/test_teardown_after_failed_setup.py diff --git a/RLTest/__main__.py b/RLTest/__main__.py index f9d783e..3367afa 100644 --- a/RLTest/__main__.py +++ b/RLTest/__main__.py @@ -745,8 +745,10 @@ def _runTest(self, test, numberOfAssertionFailed=0, prefix='', before=lambda x=N after_func = after hasException = False + setup_ok = False try: before_func() + setup_ok = True fn() passed = True except unittest.SkipTest: @@ -769,7 +771,23 @@ def _runTest(self, test, numberOfAssertionFailed=0, prefix='', before=lambda x=N hasException = True passed = False finally: - after_func() + # A tearDown written against a successful setUp (e.g. one that + # reads self.env) will itself raise if setUp never got that far. + # Skip it in that case, and otherwise route a tearDown failure + # through handleFailure like any other test failure: letting it + # escape here bypasses handleFailure and kills the whole worker + # process instead of just failing this one test. + if setup_ok: + try: + after_func() + except Exception as teardown_err: + if self.args.exit_on_failure: + raise + + self.handleFailure(testFullName=testFullName, exception=teardown_err, + prefix=msgPrefix, testname=test.name, env=self.currEnv) + hasException = True + passed = False numFailed = 0 if self.currEnv: diff --git a/tests/unit/test_teardown_after_failed_setup.py b/tests/unit/test_teardown_after_failed_setup.py new file mode 100644 index 0000000..87f56a6 --- /dev/null +++ b/tests/unit/test_teardown_after_failed_setup.py @@ -0,0 +1,79 @@ +"""Regression test for a worker crash when tearDown assumes setUp succeeded. + +RLTest._runTest ran a class-based test's tearDown unconditionally in a bare +``finally``, even when setUp itself raised. A tearDown written against a +successful setUp (e.g. one that calls ``self.env.flush()``) then raises its +own exception -- typically AttributeError, since the attribute setUp would +have assigned was never set. That second exception escaped _runTest uncaught, +which in the parallel coordinator kills the whole worker process instead of +just failing the one test. +""" + +import argparse +from unittest import TestCase + +from RLTest.__main__ import RLTest +from RLTest.loader import TestMethod + + +def _make_rltest(): + # Bypass __init__ (argparse over sys.argv, config-file lookup): _runTest + # only touches self.args.exit_on_failure, self.currEnv and self.testsFailed. + rl = RLTest.__new__(RLTest) + rl.args = argparse.Namespace(exit_on_failure=False, stop_on_failure=False, + interactive_debugger=False) + rl.currEnv = None + rl.testsFailed = {} + return rl + + +class _EnvUser: + """Mimics a test class whose setUp fails before assigning self.env.""" + + def setUp(self): + raise ConnectionError('simulated env startup failure') + + def tearDown(self): + # A real test class does this unconditionally; if setUp never ran to + # completion, self.env does not exist. + self.env.flush() + + def test_something(self): + raise AssertionError('should never run: setUp already failed') + + +class TestTearDownAfterFailedSetup(TestCase): + + def test_failed_setup_skips_teardown_without_crashing(self): + rl = _make_rltest() + instance = _EnvUser() + test = TestMethod(instance.test_something, name='fake:test_something') + + # Must not raise: a failed setUp should be reported like any other + # test failure, not escape as an unhandled exception. + numFailed = rl._runTest(test, before=instance.setUp, after=instance.tearDown) + + self.assertEqual(numFailed, 1) + self.assertIn('fake:test_something', rl.testsFailed) + self.assertIn('simulated env startup failure', str(rl.testsFailed['fake:test_something'])) + + def test_teardown_failure_after_successful_test_is_reported_not_raised(self): + rl = _make_rltest() + + class _BrokenTeardown: + def setUp(self): + pass + + def tearDown(self): + raise RuntimeError('teardown blew up') + + def test_something(self): + pass + + instance = _BrokenTeardown() + test = TestMethod(instance.test_something, name='fake:test_something2') + + rl._runTest(test, before=instance.setUp, after=instance.tearDown) + + self.assertIn('fake:test_something2', rl.testsFailed) + self.assertIn('teardown blew up', str(rl.testsFailed['fake:test_something2']))