gh-102812: unittest's addError/addFailure/addSubTest accept an exception instance in addition to (typ, val, tb) as paramter #102814

iritkatriel · 2023-03-18T13:46:29Z

Issue: unittest's addError, addFailure, addSubTest should accept exception instance or tuple #102812

…exception instance in addition to (typ, val, tb) as paramter

iritkatriel · 2023-03-18T14:22:02Z

Lib/unittest/case.py

        else:
-            result.addError(test, exc_info)
+            result.addError(test, exc)


Maybe I should revert this change, in case there are use-defined subclasses of TestResult that expect the tuple?

Yeah, I think the folks still using unittest are often in interesting and subtle places (Twisted's trial springs to mind...) so I think we really want to make sure 100% backwards compatibility (even in internal APIs sigh) is preserved if at all possible.

I'm curious: where are you finding yourself calling addError and friends in your code?

I'm not calling addError in my code. We're trying to remove the (typ, exc, tb) triplet from the language. The interpreter doesn't need it anymore, it only exists for backwards compatibility.

I made this a draft PR, it need more thought. If we can't make unittest use the new APIs then I'm not sure there is a point committing this. We would need a plan for how to migrate off the triplet.

This is related to the PEP707 discussion, so we could wait to see what comes out of that.

iritkatriel · 2023-03-18T14:22:14Z

Lib/unittest/case.py

-            except _UnexpectedSuccess:
-                result.addFailure(self, sys.exc_info())
+            except _UnexpectedSuccess as e:
+                result.addFailure(self, e)


iritkatriel · 2023-03-18T14:22:29Z

Lib/unittest/suite.py

                        self._createClassOrModuleLevelException(
                                result, exc_info[1], 'setUpClass', className,
-                                info=exc_info)
+                                err=exc)


terryjreedy · 2023-03-18T19:27:09Z

Doc/whatsnew/3.12.rst

+:meth:`~unittest.TestResult.addSubTest` now accept either an exception
+instance or a ``(typ, val, tb)`` tuple. (Contributed by Irit Katriel in


Make clear what is new while being clear that still only 1 is passed.

Suggested change

:meth:`~unittest.TestResult.addSubTest` now accept either an exception

instance or a ``(typ, val, tb)`` tuple. (Contributed by Irit Katriel in

:meth:`~unittest.TestResult.addSubTest` now accept an exception instead of

a ``(typ, val, tb)`` tuple. (Contributed by Irit Katriel in

Do you want to say that an exception is in some sense preferred?

I think "instead of" may sound like the old format is no longer accepted. But agree I would emphasise what is new and what is old.

terryjreedy · 2023-03-18T19:39:13Z

Lib/unittest/runner.py

+                if isinstance(err, subtest.failureException):
                    self._write_status(subtest, "FAIL")
                else:
                    self._write_status(subtest, "ERROR")


Factor out minimal difference in the two branches depending on the condition.

Suggested change

if isinstance(err, subtest.failureException):

self._write_status(subtest, "FAIL")

else:

self._write_status(subtest, "ERROR")

status = "FAIL" if isinstance(err, subtest.failureException) else "ERROR"

self._write_status(subtest, status)

There might be a better word than 'status'. Note: if the statement were not already being edited, this would be an unrelated change and I would not suggest it here. But since you are ...

terryjreedy · 2023-03-18T19:45:51Z

Lib/unittest/runner.py

+                if isinstance(err, subtest.failureException):
                    self.stream.write('F')
                else:
                    self.stream.write('E')


Suggested change

if isinstance(err, subtest.failureException):

self.stream.write('F')

else:

self.stream.write('E')

dot = 'F' if isinstance(err, subtest.failureException) else 'E'

self.stream.write(dot)

terryjreedy · 2023-03-18T19:55:28Z

Lib/unittest/suite.py

        error = _ErrorHolder(errorName)
        addSkip = getattr(result, 'addSkip', None)
        if addSkip is not None and isinstance(exception, case.SkipTest):
            addSkip(error, str(exception))
        else:
-            if not info:
-                result.addError(error, sys.exc_info())
+            if not err:


It is not clear why the assert it needed, but with it, a possible refactoring (which cannot be marked as a multi-line suggestion).

if not err: err = sys.exc_info()[1] else: assert not isinstance(err, tuple) result.addError(error, err)

terryjreedy · 2023-03-18T19:56:47Z

Misc/NEWS.d/next/Library/2023-03-18-13-09-35.gh-issue-102812.IcPYBs.rst

+:meth:`~unittest.TestResult.addSubTest` now accept either an exception
+instance or a ``(typ, val, tb)`` tuple.


Suggested change

:meth:`~unittest.TestResult.addSubTest` now accept either an exception

instance or a ``(typ, val, tb)`` tuple.

:meth:`~unittest.TestResult.addSubTest` now accept an exception instance

instead of a ``(typ, val, tb)`` tuple.

terryjreedy

My turn to ping ;-). Too late for 3.12 I guess, hence new suggestion(s). Please accept the suggestions you are sure of, and resolve when appropriate. I fixed the merge conflict (easy).

terryjreedy · 2023-06-11T22:48:05Z

Doc/library/unittest.rst


      The default implementation appends a tuple ``(test, formatted_err)`` to
      the instance's :attr:`errors` attribute, where *formatted_err* is a
      formatted traceback derived from *err*.

+      .. versionchanged:: 3.12


Suggested change

.. versionchanged:: 3.12

.. versionchanged:: 3.13

terryjreedy · 2023-06-11T22:48:20Z

Doc/library/unittest.rst


      The default implementation appends a tuple ``(test, formatted_err)`` to
      the instance's :attr:`failures` attribute, where *formatted_err* is a
      formatted traceback derived from *err*.

+      .. versionchanged:: 3.12


Suggested change

.. versionchanged:: 3.12

.. versionchanged:: 3.13

terryjreedy · 2023-06-11T22:48:36Z

Doc/library/unittest.rst


      The default implementation does nothing when the outcome is a
      success, and records subtest failures as normal failures.

      .. versionadded:: 3.4

-   .. method:: addDuration(test, elapsed)
+      .. versionchanged:: 3.12


Suggested change

.. versionchanged:: 3.12

.. versionchanged:: 3.13

iritkatriel · 2023-06-14T13:52:18Z

Thanks, @terryjreedy . I have some doubts about this PR because I think there might be user-defined subclasses of unittest that override these functions (essentially callbacks expecting (typ, exc, tb) triplets). I'm not sure how to move us on from that.

iritkatriel · 2023-11-12T20:03:24Z

Closing because I think this is more complicated than I thought. (See previous comment).

pythongh-102812: unittest's addError/addFailure/addSubTest accept an …

f88a4ce

…exception instance in addition to (typ, val, tb) as paramter

iritkatriel requested a review from cjw296 as a code owner March 18, 2023 13:46

bedevere-bot mentioned this pull request Mar 18, 2023

unittest's addError, addFailure, addSubTest should accept exception instance or tuple #102812

Closed

bedevere-bot added the awaiting core review label Mar 18, 2023

iritkatriel commented Mar 18, 2023

View reviewed changes

terryjreedy reviewed Mar 18, 2023

View reviewed changes

iritkatriel marked this pull request as draft March 19, 2023 11:14

cjw296 removed their request for review June 9, 2023 13:02

terryjreedy added 2 commits June 11, 2023 18:40

Merge branch 'main' into unittest-exc

eb90f80

Update unittest.rst

4b47a6b

terryjreedy reviewed Jun 11, 2023

View reviewed changes

iritkatriel closed this Nov 12, 2023

		:meth:`~unittest.TestResult.addSubTest` now accept either an exception
		instance or a ``(typ, val, tb)`` tuple. (Contributed by Irit Katriel in

		:meth:`~unittest.TestResult.addSubTest` now accept either an exception
		instance or a ``(typ, val, tb)`` tuple.

Uh oh!

gh-102812: unittest's addError/addFailure/addSubTest accept an exception instance in addition to (typ, val, tb) as paramter #102814

gh-102812: unittest's addError/addFailure/addSubTest accept an exception instance in addition to (typ, val, tb) as paramter #102814

Uh oh!

Conversation

iritkatriel commented Mar 18, 2023 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

terryjreedy Mar 18, 2023 • 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!

terryjreedy Mar 18, 2023 • 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!

terryjreedy 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!

iritkatriel commented Jun 14, 2023

Uh oh!

iritkatriel commented Nov 12, 2023

Uh oh!

Uh oh!

iritkatriel commented Mar 18, 2023 •

edited by bedevere-bot

Loading

terryjreedy Mar 18, 2023 •

edited

Loading

terryjreedy Mar 18, 2023 •

edited

Loading

terryjreedy left a comment •

edited

Loading