gh-134079: Add `addCleanup`, `enterContext` and `doCleanups` to `unittest.subTest` and tests by ArunRawat404 · Pull Request #134318 · python/cpython

ArunRawat404 · 2025-05-20T08:16:49Z

Summary

gh-134079: Add addCleanup, enterContext and doCleanups to unittest.subTest and add corresponding tests

Description

This PR implements support for addCleanup, enterContext, and doCleanups in unittest.subTest() contexts, as discussed in issue-134079. This enables users to register cleanups or use context managers inside a with self.subTest(): block, similar to how it's done in TestCase

Tests

Added tests in test_runner.py to validate:

Cleanup order (LIFO)
Cleanup on failure
enterContext() behavior on success/failure
Exception safety in context manager entry and exit

I'm still learning the internals of CPython and writing tests, so please don't hesitate to suggest improvements. I'd really appreciate any feedback.

python-cla-bot · 2025-05-20T08:16:54Z

All commit authors signed the Contributor License Agreement.

bedevere-app · 2025-05-20T08:16:55Z

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

picnixz

cc @encukou

picnixz · 2025-05-20T16:00:01Z

Lib/unittest/case.py

        return "{} {}".format(self.test_case, self._subDescription())
+
+
+class _SubTestCleanupHelper():


Suggested change

class _SubTestCleanupHelper():

class _SubTestCleanupHelper:

picnixz · 2025-05-20T16:01:11Z

Lib/unittest/case.py

+    def _callCleanup(self, function, /, *args, **kwargs):
+        function(*args, **kwargs)


Suggested change

def _callCleanup(self, function, /, *args, **kwargs):

function(*args, **kwargs)

@staticmethod

def _callCleanup(function, /, *args, **kwargs):

function(*args, **kwargs)

I don't think we need to hold a reference to self so a staticmethod can be used but OTOH, I don't know if there is a world where we want to be able to subclass this one in the future.

picnixz · 2025-05-20T16:01:24Z

Lib/test/test_unittest/test_runner.py

+        if hasattr(TestSubTestCleanups, '_active_instance'):
+            del TestSubTestCleanups._active_instance



Suggested change

if hasattr(TestSubTestCleanups, '_active_instance'):

del TestSubTestCleanups._active_instance

if hasattr(TestSubTestCleanups, '_active_instance'):

del TestSubTestCleanups._active_instance

Can you explain this one i don't see any difference

We use 2 blank lines to separate toplevel classes/functions and 1 line for methods.

picnixz · 2025-05-20T16:01:49Z

Lib/test/test_unittest/test_runner.py

+        ]
+        self.assertEqual(self.events, expected_events)
+
+    def tearDown(self):


Can you put the tearDown() after the setUp() instead? it'll be easier to maintain and read.

encukou

Thanks! This looks great so far. Do you want to write the documentation as well?

encukou · 2025-05-29T12:07:20Z

Lib/test/test_unittest/test_runner.py

+    def test_addCleanup_operation_and_LIFO_order(self):
+        class MyTests(unittest.TestCase):
+            def runTest(inner_self):
+                events = TestSubTestCleanups._get_events()
+                recorder = TestSubTestCleanups._active_instance._record_event
+                with inner_self.subTest() as sub:
+                    events.append("subtest_body_start")
+                    sub.addCleanup(recorder, "cleanup_2_args", "arg")
+                    sub.addCleanup(recorder, "cleanup_1")
+                    events.append("subtest_body_end")
+
+        MyTests().run()
+
+        expected_events = [
+            "subtest_body_start",
+            "subtest_body_end",
+            "cleanup_1",
+            "cleanup_2_args",
+        ]
+        self.assertEqual(self.events, expected_events)


There should be no need for _active_events_list and _get_events and _record_event.

Suggested change

def test_addCleanup_operation_and_LIFO_order(self):

class MyTests(unittest.TestCase):

def runTest(inner_self):

events = TestSubTestCleanups._get_events()

recorder = TestSubTestCleanups._active_instance._record_event

with inner_self.subTest() as sub:

events.append("subtest_body_start")

sub.addCleanup(recorder, "cleanup_2_args", "arg")

sub.addCleanup(recorder, "cleanup_1")

events.append("subtest_body_end")

MyTests().run()

expected_events = [

"subtest_body_start",

"subtest_body_end",

"cleanup_1",

"cleanup_2_args",

]

self.assertEqual(self.events, expected_events)

def test_addCleanup_operation_and_LIFO_order(self):

events = []

class MyTests(unittest.TestCase):

def runTest(inner_self):

def record(event_name, *args, **kwargs):

events.append((event_name, args, kwargs))

with inner_self.subTest() as sub:

events.append("subtest_body_start")

sub.addCleanup(record, "cleanup_2_args", "pos", kw="kwd")

sub.addCleanup(record, "cleanup_1")

events.append("subtest_body_end")

MyTests().run()

expected_events = [

"subtest_body_start",

"subtest_body_end",

("cleanup_1", (), {}),

("cleanup_2_args", ("pos", ), {"kw": "kwd"}),

]

self.assertEqual(events, expected_events)

encukou · 2025-05-29T12:07:59Z

Lib/unittest/case.py

+class _SubTestCleanupHelper:
+    """
+    Helper class to manage cleanups and context managers inside subTest blocks,
+    without exposing full TestCase functionality.
+    """


Nitpick: Perhaps Context would be a better name rather than Helper:

Suggested change

class _SubTestCleanupHelper:

"""

Helper class to manage cleanups and context managers inside subTest blocks,

without exposing full TestCase functionality.

"""

class _SubTestContext:

"""

Helper class to manage cleanups and context managers inside subTest blocks,

without exposing full TestCase functionality.

"""

encukou · 2025-05-29T12:08:04Z

Lib/unittest/case.py

+        If successful, also adds its __exit__ method as a cleanup
+        function and returns the result of the __enter__ method.
+        """
+        return _enter_context(cm, self.addCleanup)


Could you add a flag argument to _enter_context to prevent adding the "enterAsyncContext" suggestion?
An async version of this is probably better left to a separate PR.

encukou · 2025-05-29T12:08:07Z

Lib/unittest/case.py

+        outcome = self._outcome or _Outcome()
+        while self._cleanups:
+            function, args, kwargs = self._cleanups.pop()
+            if hasattr(outcome, 'testPartExecutor'):


Is there any way the outcome would not have testPartExecutor?

encukou · 2025-05-29T12:08:09Z

Lib/unittest/case.py

+            function, args, kwargs = self._cleanups.pop()
+            if hasattr(outcome, 'testPartExecutor'):
+                with outcome.testPartExecutor(self._subtest, subTest=True):
+                    self._callCleanup(function, *args, **kwargs)


Can this be function(*args, **kwargs)? I see no need for an extra method.

ArunRawat404 added 2 commits May 18, 2025 21:17

feat: add _SubTestCleanupHelper for managing cleanups/context in subTest

d12ab40

test: add unit tests for subTest cleanup and context manager behavior

0edd1be

bedevere-app bot mentioned this pull request May 20, 2025

Add addCleanup to unittest.subTest #134079

Open

bedevere-app bot added the awaiting review label May 20, 2025

picnixz reviewed May 20, 2025

View reviewed changes

ArunRawat404 added 2 commits May 22, 2025 19:09

fix: update some basic code

c9aad93

Add NEWS entry

f09d6f4

encukou reviewed May 29, 2025

View reviewed changes

serhiy-storchaka self-requested a review June 4, 2025 10:58

		return "{} {}".format(self.test_case, self._subDescription())


		class _SubTestCleanupHelper():

		def _callCleanup(self, function, /, args, *kwargs):
		function(args, *kwargs)

		if hasattr(TestSubTestCleanups, '_active_instance'):
		del TestSubTestCleanups._active_instance

Uh oh!

Conversation

ArunRawat404 commented May 20, 2025

Summary

Description

Tests

Uh oh!

python-cla-bot bot commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-app bot commented May 20, 2025

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArunRawat404 May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

picnixz May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

encukou left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

python-cla-bot bot commented May 20, 2025 •

edited

Loading

ArunRawat404 May 21, 2025 •

edited

Loading

picnixz May 22, 2025 •

edited

Loading

encukou left a comment •

edited

Loading