Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unittest.TestResult fails with exception deriving from frozen dataclass #117211

Closed
thomascellerier opened this issue Mar 25, 2024 · 9 comments
Closed
Labels
type-bug An unexpected behavior, bug, or error

Comments

@thomascellerier
Copy link
Contributor

thomascellerier commented Mar 25, 2024

Bug report

Bug description:

The unittest.TestResult class assumes that it can assign to the __traceback__ attribute of the exception.
This will not work with an exception that is also a frozen dataclass.

I have not been able to create a simple reproducer for this but as far as I can see the offending code is still in main.
See https://github.com/python/cpython/blob/v3.13.0a5/Lib/unittest/result.py#L226

For example:

from dataclasses import dataclass

@dataclass(frozen=True)
class FrozenException(Exception):
    message: str

Here is the traceback using python3.10.12 in an ubuntu based docker container.

Traceback (most recent call last):
  File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/build/project/tests/__main__.py", line 15, in <module>
    run_unit_tests()
  File "/build/project/tests/__init__.py", line 46, in run_unit_tests
    run_tests(UNIT_TESTS_PATH)
  File "/build/project/tests/__init__.py", line 40, in run_tests
    result = runner.run(suite)
  File "/usr/lib/python3.10/unittest/runner.py", line 184, in run
    test(result)
  File "/usr/lib/python3.10/unittest/suite.py", line 84, in __call__
    return self.run(*args, **kwds)
  File "/usr/lib/python3.10/unittest/suite.py", line 122, in run
    test(result)
  File "/usr/lib/python3.10/unittest/suite.py", line 84, in __call__
    return self.run(*args, **kwds)
  File "/usr/lib/python3.10/unittest/suite.py", line 122, in run
    test(result)
  File "/usr/lib/python3.10/unittest/suite.py", line 84, in __call__
    return self.run(*args, **kwds)
  File "/usr/lib/python3.10/unittest/suite.py", line 122, in run
    test(result)
  File "/usr/lib/python3.10/unittest/case.py", line 650, in __call__
    return self.run(*args, **kwds)
  File "/usr/lib/python3.10/unittest/async_case.py", line 159, in run
    return super().run(result)
  File "/usr/lib/python3.10/unittest/case.py", line 599, in run
    self._feedErrorsToResult(result, outcome.errors)
  File "/usr/lib/python3.10/unittest/case.py", line 518, in _feedErrorsToResult
    result.addError(test, exc_info)
  File "/usr/lib/python3.10/unittest/runner.py", line 68, in addError
    super(TextTestResult, self).addError(test, err)
  File "/usr/lib/python3.10/unittest/result.py", line 17, in inner
    return method(self, *args, **kw)
  File "/usr/lib/python3.10/unittest/result.py", line 115, in addError
    self.errors.append((test, self._exc_info_to_string(err, test)))
  File "/usr/lib/python3.10/unittest/result.py", line 176, in _exc_info_to_string
    tb = self._clean_tracebacks(exctype, value, tb, test)
  File "/usr/lib/python3.10/unittest/result.py", line 215, in _clean_tracebacks
    value.__traceback__ = tb
  File "<string>", line 4, in __setattr__
dataclasses.FrozenInstanceError: cannot assign to field '__traceback__'

CPython versions tested on:

3.10

Operating systems tested on:

Linux

@thomascellerier thomascellerier added the type-bug An unexpected behavior, bug, or error label Mar 25, 2024
@sobolevn sobolevn changed the title unittest.TestResult crash with exception deriving from frozen dataclass unittest.TestResult fails with exception deriving from frozen dataclass Mar 25, 2024
@sobolevn
Copy link
Member

Since there's no reproducer right now, I am not fully convinced that this is a bug. Exceptions are expected to be mutable. If you drop some parts of an API from the core CPython object, it will raise an expected error.

Do you agree? :)

@thomascellerier
Copy link
Contributor Author

I managed to reproduce it now, run with python3 -m unittest test.py, with test.py being:

from dataclasses import dataclass
import unittest


class TestFrozenDataclassException(unittest.IsolatedAsyncioTestCase):
    async def test_frozen_dataclass_exception(self):
        @dataclass(frozen=True)
        class FrozenException(Exception):
            value: int

        try:
            raise FrozenException(value=123)
        except:
            raise ValueError("foo")

Which results in:

$ python3 --version
Python 3.11.8
$ python3 -m unittest test.py
Traceback (most recent call last):
 File "/home/thomas/Projects/themachine/test.py", line 12, in test_frozen_dataclass_exception
   raise FrozenException(value=123)
test.TestFrozenDataclassException.test_frozen_dataclass_exception.<locals>.FrozenException

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
 File "/usr/lib/python3.11/unittest/case.py", line 57, in testPartExecutor
   yield
 File "/usr/lib/python3.11/unittest/case.py", line 623, in run
   self._callTestMethod(testMethod)
 File "/usr/lib/python3.11/unittest/async_case.py", line 90, in _callTestMethod
   if self._callMaybeAsync(method) is not None:
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/unittest/async_case.py", line 112, in _callMaybeAsync
   return self._asyncioRunner.run(
          ^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/asyncio/runners.py", line 118, in run
   return self._loop.run_until_complete(task)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/asyncio/base_events.py", line 654, in run_until_complete
   return future.result()
          ^^^^^^^^^^^^^^^
 File "/home/thomas/Projects/themachine/test.py", line 14, in test_frozen_dataclass_exception
   raise ValueError("foo")
ValueError: foo

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
 File "<frozen runpy>", line 198, in _run_module_as_main
 File "<frozen runpy>", line 88, in _run_code
 File "/usr/lib/python3.11/unittest/__main__.py", line 18, in <module>
   main(module=None)
 File "/usr/lib/python3.11/unittest/main.py", line 102, in __init__
   self.runTests()
 File "/usr/lib/python3.11/unittest/main.py", line 274, in runTests
   self.result = testRunner.run(self.test)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/unittest/runner.py", line 217, in run
   test(result)
 File "/usr/lib/python3.11/unittest/suite.py", line 84, in __call__
   return self.run(*args, **kwds)
          ^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/unittest/suite.py", line 122, in run
   test(result)
 File "/usr/lib/python3.11/unittest/suite.py", line 84, in __call__
   return self.run(*args, **kwds)
          ^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/unittest/suite.py", line 122, in run
   test(result)
 File "/usr/lib/python3.11/unittest/suite.py", line 84, in __call__
   return self.run(*args, **kwds)
          ^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/unittest/suite.py", line 122, in run
   test(result)
 File "/usr/lib/python3.11/unittest/case.py", line 678, in __call__
   return self.run(*args, **kwds)
          ^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/unittest/async_case.py", line 131, in run
   return super().run(result)
          ^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/unittest/case.py", line 622, in run
   with outcome.testPartExecutor(self):
 File "/usr/lib/python3.11/contextlib.py", line 158, in __exit__
   self.gen.throw(typ, value, traceback)
 File "/usr/lib/python3.11/unittest/case.py", line 74, in testPartExecutor
   _addError(self.result, test_case, exc_info)
 File "/usr/lib/python3.11/unittest/case.py", line 99, in _addError
   result.addError(test, exc_info)
 File "/usr/lib/python3.11/unittest/runner.py", line 98, in addError
   super(TextTestResult, self).addError(test, err)
 File "/usr/lib/python3.11/unittest/result.py", line 17, in inner
   return method(self, *args, **kw)
          ^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/unittest/result.py", line 115, in addError
   self.errors.append((test, self._exc_info_to_string(err, test)))
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/unittest/result.py", line 176, in _exc_info_to_string
   tb = self._clean_tracebacks(exctype, value, tb, test)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/python3.11/unittest/result.py", line 214, in _clean_tracebacks
   value.__traceback__ = tb
   ^^^^^^^^^^^^^^^^^^^
 File "<string>", line 4, in __setattr__
dataclasses.FrozenInstanceError: cannot assign to field '__traceback__'

@thomascellerier
Copy link
Contributor Author

Exceptions are expected to be mutable.

I was trying to find if that was documented anywhere but could not find any definitive documentation, maybe I'm not looking at the right place though.

The python3.11+ exception notes feature certainly relies on mutability as well but I could not find anything in the PEP describing what happens if an exception overrides __setattr__ in way that forbids adding and/or setting an attribute.

Regardless, I would expect a test framework to handle that in a way that only fails the offending test and not the entire test run.
It could even point to the root issue, e.g. "Exception subclasses should be mutable".

@sobolevn
Copy link
Member

Well, if something is not documented as immutable, it is mutable by default :)
That's how Python is designed.

However, I think that this part:

Regardless, I would expect a test framework to handle that in a way that only fails the offending test and not the entire test run.

makes sense. Probably error reporting here can be better (for all cases, not just this one).

@thomascellerier
Copy link
Contributor Author

Same in main, but with much nicer colored output :)


thomas@thomas-xps13:~/Projects/cpython$ ./python --version
Python 3.13.0a5+
thomas@thomas-xps13:~/Projects/cpython$ ./python -m unittest test.py
Traceback (most recent call last):
  File "/home/thomas/Projects/cpython/test.py", line 12, in test_frozen_dataclass_exception
    raise FrozenException(value=123)
test.TestFrozenDataclassException.test_frozen_dataclass_exception.<locals>.FrozenException

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/thomas/Projects/cpython/Lib/unittest/case.py", line 58, in testPartExecutor
    yield
  File "/home/thomas/Projects/cpython/Lib/unittest/case.py", line 634, in run
    self._callTestMethod(testMethod)
    ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/async_case.py", line 93, in _callTestMethod
    if self._callMaybeAsync(method) is not None:
       ~~~~~~~~~~~~~~~~~~~~^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/async_case.py", line 115, in _callMaybeAsync
    return self._asyncioRunner.run(
           ~~~~~~~~~~~~~~~~~~~~~~~^
        func(*args, **kwargs),
        ^^^^^^^^^^^^^^^^^^^^^^
        context=self._asyncioTestContext,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/home/thomas/Projects/cpython/Lib/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
  File "/home/thomas/Projects/cpython/Lib/asyncio/base_events.py", line 721, in run_until_complete
    return future.result()
           ~~~~~~~~~~~~~^^
  File "/home/thomas/Projects/cpython/test.py", line 14, in test_frozen_dataclass_exception
    raise ValueError("foo")
ValueError: foo

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/thomas/Projects/cpython/Lib/runpy.py", line 198, in _run_module_as_main
    return _run_code(code, main_globals, None,
           ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
                     "__main__", mod_spec)
                     ^^^^^^^^^^^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/runpy.py", line 88, in _run_code
    exec(code, run_globals)
    ~~~~^^^^^^^^^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/__main__.py", line 18, in <module>
    main(module=None)
    ~~~~^^^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/main.py", line 104, in __init__
    self.runTests()
    ~~~~~~~~~~~~~^^
  File "/home/thomas/Projects/cpython/Lib/unittest/main.py", line 270, in runTests
    self.result = testRunner.run(self.test)
                  ~~~~~~~~~~~~~~^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/runner.py", line 240, in run
    test(result)
    ~~~~^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/suite.py", line 84, in __call__
    return self.run(*args, **kwds)
           ~~~~~~~~^^^^^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/suite.py", line 122, in run
    test(result)
    ~~~~^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/suite.py", line 84, in __call__
    return self.run(*args, **kwds)
           ~~~~~~~~^^^^^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/suite.py", line 122, in run
    test(result)
    ~~~~^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/suite.py", line 84, in __call__
    return self.run(*args, **kwds)
           ~~~~~~~~^^^^^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/suite.py", line 122, in run
    test(result)
    ~~~~^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/case.py", line 690, in __call__
    return self.run(*args, **kwds)
           ~~~~~~~~^^^^^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/async_case.py", line 134, in run
    return super().run(result)
           ~~~~~~~~~~~^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/case.py", line 633, in run
    with outcome.testPartExecutor(self):
        self._callTestMethod(testMethod)
  File "/home/thomas/Projects/cpython/Lib/contextlib.py", line 162, in __exit__
    self.gen.throw(value)
    ~~~~~~~~~~~~~~^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/case.py", line 75, in testPartExecutor
    _addError(self.result, test_case, exc_info)
    ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/case.py", line 100, in _addError
    result.addError(test, exc_info)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/runner.py", line 101, in addError
    super(TextTestResult, self).addError(test, err)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/result.py", line 17, in inner
    return method(self, *args, **kw)
           ~~~~~~^^^^^^^^^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/result.py", line 116, in addError
    self.errors.append((test, self._exc_info_to_string(err, test)))
                              ~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/result.py", line 188, in _exc_info_to_string
    tb = self._clean_tracebacks(exctype, value, tb, test)
         ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/result.py", line 226, in _clean_tracebacks
    value.__traceback__ = tb
    ^^^^^^^^^^^^^^^^^^^
  File "<string>", line 4, in __setattr__
dataclasses.FrozenInstanceError: cannot assign to field '__traceback__'
thomas@thomas-xps13:~/Projects/cpython$ git rev-parse HEAD
eebea7e515462b503632ada74923ec3246599c9c

@AlexWaygood
Copy link
Member

I'm going to close this as a duplicate of #99856, as it's the same underlying issue that's being discussed (many places in the stdlib assume that exceptions are mutable; using frozen=True for Exception subclasses is generally a bad idea)

@AlexWaygood AlexWaygood closed this as not planned Won't fix, can't repro, duplicate, stale Mar 25, 2024
@thomascellerier
Copy link
Contributor Author

This small patch avoids this problem, though of course I'm not sure of any other implications here:

thomas@thomas-xps13:~/Projects/cpython$ ./python -m unittest test.py
EE
======================================================================
ERROR: test_frozen_dataclass_exception (test.TestFrozenDataclassException.test_frozen_dataclass_exception)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/thomas/Projects/cpython/test.py", line 12, in test_frozen_dataclass_exception
    raise FrozenException(value=123)
test.TestFrozenDataclassException.test_frozen_dataclass_exception.<locals>.FrozenException

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/thomas/Projects/cpython/Lib/unittest/async_case.py", line 93, in _callTestMethod
    if self._callMaybeAsync(method) is not None:
       ~~~~~~~~~~~~~~~~~~~~^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/async_case.py", line 115, in _callMaybeAsync
    return self._asyncioRunner.run(
           ~~~~~~~~~~~~~~~~~~~~~~~^
        func(*args, **kwargs),
        ^^^^^^^^^^^^^^^^^^^^^^
        context=self._asyncioTestContext,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/home/thomas/Projects/cpython/Lib/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
  File "/home/thomas/Projects/cpython/Lib/asyncio/base_events.py", line 721, in run_until_complete
    return future.result()
           ~~~~~~~~~~~~~^^
  File "/home/thomas/Projects/cpython/test.py", line 14, in test_frozen_dataclass_exception
    raise ValueError("foo")
ValueError: foo

======================================================================
ERROR: test_other (test.TestFrozenDataclassException.test_other)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/thomas/Projects/cpython/Lib/unittest/async_case.py", line 93, in _callTestMethod
    if self._callMaybeAsync(method) is not None:
       ~~~~~~~~~~~~~~~~~~~~^^^^^^^^
  File "/home/thomas/Projects/cpython/Lib/unittest/async_case.py", line 120, in _callMaybeAsync
    return self._asyncioTestContext.run(func, *args, **kwargs)
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
TypeError: TestFrozenDataclassException.test_other() takes 0 positional arguments but 1 was given

----------------------------------------------------------------------
Ran 2 tests in 0.015s

FAILED (errors=2)
thomas@thomas-xps13:~/Projects/cpython$ git diff
diff --git a/Lib/unittest/result.py b/Lib/unittest/result.py
index 3ace0a5..da32563 100644
--- a/Lib/unittest/result.py
+++ b/Lib/unittest/result.py
@@ -223,7 +223,11 @@ def _clean_tracebacks(self, exctype, value, tb, test):
                 ret = tb
                 first = False
             else:
-                value.__traceback__ = tb
+                try:
+                    value.__traceback__ = tb
+                except AttributeError:
+                    # Attribute is not settable in exception class
+                    pass
 
             if value is not None:
                 for c in (value.__cause__, value.__context__):

@thomascellerier
Copy link
Contributor Author

I'm not sure this is exactly the same as the other case ticket since we are speaking about a test runner here.
Ideally, errors should be isolates as much as possible to single test cases.

Would it make sense to add a note either in dataclasses or Exception documentation to explicitly mention that Exception subclasses are expected to be mutable and should thus not be frozen (from the dataclass perspective).

@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 25, 2024

I'm not sure this is exactly the same as the other case ticket since we are speaking about a test runner here.
Ideally, errors should be isolates as much as possible to single test cases.

The context in which the issue is arising is different, but it's the same underlying issue: many places in the stdlib and third-party libraries (including the test runners provided by the stdlib) depend on being able to mutate attributes of exception instances; you'll break a lot of assumptions in a lot of places if you try to create a frozen exception class.

Would it make sense to add a note either in dataclasses or Exception documentation to explicitly mention that Exception subclasses are expected to be mutable and should thus not be frozen (from the dataclass perspective).

I definitely agree that we need to document this better! Let's discuss on the other issue where it would be best to add that documentation, though 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants