Add some miscellaneous tests #659
Conversation
| else: | ||
| mod = typing | ||
| class Foo: ... | ||
| Alias = mod._SpecialGenericAlias(Foo, 2) |
There was a problem hiding this comment.
Directly testing _SpecialGenericAlias feels a little funny, but I can't think of a different way to test the lines that depend on defaults being empty, since all of the public instances have defaults set
There was a problem hiding this comment.
I guess the alternative is to remove the dead code from typing_extensions.py?
diff --git a/src/typing_extensions.py b/src/typing_extensions.py
index c2ecc2f..481298c 100644
--- a/src/typing_extensions.py
+++ b/src/typing_extensions.py
@@ -555,7 +555,8 @@ else:
class _SpecialGenericAlias(typing._SpecialGenericAlias, _root=True):
- def __init__(self, origin, nparams, *, inst=True, name=None, defaults=()):
+ def __init__(self, origin, nparams, *, defaults, inst=True, name=None):
+ assert defaults, "Must always specify a non-empty sequence for `defaults`"
super().__init__(origin, nparams, inst=inst, name=name)
self._defaults = defaults
@@ -573,18 +574,14 @@ else:
msg = "Parameters to generic types must be types."
params = tuple(typing._type_check(p, msg) for p in params)
if (
- self._defaults
- and len(params) < self._nparams
+ len(params) < self._nparams
and len(params) + len(self._defaults) >= self._nparams
):
params = (*params, *self._defaults[len(params) - self._nparams:])
actual_len = len(params)
if actual_len != self._nparams:
- if self._defaults:
- expected = f"at least {self._nparams - len(self._defaults)}"
- else:
- expected = str(self._nparams)
+ expected = f"at least {self._nparams - len(self._defaults)}"
if not self._nparams:
raise TypeError(f"{self} is not a generic class")
raise TypeError(We only added the class in #382 to backport support for type-parameter defaults for these aliases; I'm not sure I can think of a reason why we'd use it for an alias that doesn't have type-parameter defaults.
| if sys.version_info >= (3, 10): | ||
| with self.assertRaises(TypeError): | ||
| class MyParamSpec(ParamSpec): | ||
| pass | ||
| else: | ||
| class MyParamSpec(ParamSpec): # Does not raise | ||
| pass |
There was a problem hiding this comment.
Should the Python 3.9 typing_extensions implementation raise as well?
| @skipUnless(TYPING_3_10_0, "PEP 604 has yet to be") | ||
| def test_or(self): | ||
| self.assertEqual(Self | int, Union[Self, int]) | ||
| self.assertEqual(int | Self, Union[int, Self]) | ||
|
|
||
| self.assertEqual(get_args(Self | int), (Self, int)) | ||
| self.assertEqual(get_args(int | Self), (int, Self)) |
There was a problem hiding this comment.
I only added a test for Self since this was enough to exercise the relevant lines. Would it be worth adding similar tests for other _SpecialForm instances?
There was a problem hiding this comment.
I think in this case, yes, it's generally important that | works for all these special forms on 3.10+, so it's probably good to explicitly test all of them
| def test_module_with_incomplete_sys(self): | ||
| def does_not_exist(*args): | ||
| raise AttributeError | ||
| with ( | ||
| patch("sys._getframemodulename", does_not_exist, create=True), | ||
| patch("sys._getframe", does_not_exist, create=True), | ||
| ): | ||
| X = NewType("X", int) | ||
| self.assertEqual(X.__module__, None) |
There was a problem hiding this comment.
I only added a test for NewType since this was enough to exercise the relevant lines. Would it be worth adding similar tests for other constructs that set __module__?)
There was a problem hiding this comment.
I think this is fine! We run the tests on PyPy in CI so I think we can have pretty good confidence that things like NewType work on implementations that don't have these private sys APIs. (The reason why they show up as not having coverage is we ran into some issues running the tests under coverage on PyPy, and decided it probably wasn't high-priority to figure out what the cause of that was)
| @@ -2254,6 +2254,52 @@ def test_asynccontextmanager_type_params(self): | |||
| cm2 = typing_extensions.AsyncContextManager[int, None] | |||
| self.assertEqual(get_args(cm2), (int, NoneType)) | |||
|
|
|||
| def test_setattr(self): | |||
There was a problem hiding this comment.
I'm guessing you're testing the private API directly here because you don't want to set attributes on typing_extensions.ContextManager (etc.) that would persist after this test has completed, polluting the environment for other tests? Is it worth adding a comment?
| def test_module_with_incomplete_sys(self): | ||
| def does_not_exist(*args): | ||
| raise AttributeError | ||
| with ( | ||
| patch("sys._getframemodulename", does_not_exist, create=True), | ||
| patch("sys._getframe", does_not_exist, create=True), | ||
| ): | ||
| X = NewType("X", int) | ||
| self.assertEqual(X.__module__, None) |
There was a problem hiding this comment.
I think this is fine! We run the tests on PyPy in CI so I think we can have pretty good confidence that things like NewType work on implementations that don't have these private sys APIs. (The reason why they show up as not having coverage is we ran into some issues running the tests under coverage on PyPy, and decided it probably wasn't high-priority to figure out what the cause of that was)
| def foobar2(x: list['X']): ... | ||
| if sys.version_info >= (3, 11): | ||
| self.assertEqual( | ||
| get_type_hints(foobar2, globals(), locals()), | ||
| {'x': list[int]} | ||
| ) | ||
| self.assertEqual( | ||
| get_type_hints(foobar2, globals(), locals(), include_extras=True), | ||
| {'x': list[Annotated[int, (1, 10)]]} | ||
| ) | ||
| else: # TODO: evaluate nested forward refs in Python < 3.11 | ||
| self.assertEqual( | ||
| get_type_hints(foobar2, globals(), locals()), | ||
| {'x': list['X']} | ||
| ) | ||
| self.assertEqual( | ||
| get_type_hints(foobar2, globals(), locals(), include_extras=True), | ||
| {'x': list['X']} | ||
| ) |
There was a problem hiding this comment.
I don't love it when a test asserts a different thing on one version than it does on another version. Another way to approach this might be to split the new >=3.11 assertions into a new test method that is decorated with @skipUnless(sys.version_info >= (3, 11), "TODO: evaluate nested forward refs in Python < 3.11")
| if sys.version_info >= (3, 10): | ||
| with self.assertRaises(TypeError): | ||
| class MyParamSpec(ParamSpec): | ||
| pass | ||
| else: | ||
| class MyParamSpec(ParamSpec): # Does not raise | ||
| pass |
| @skipUnless(TYPING_3_10_0, "PEP 604 has yet to be") | ||
| def test_or(self): | ||
| self.assertEqual(Self | int, Union[Self, int]) | ||
| self.assertEqual(int | Self, Union[int, Self]) | ||
|
|
||
| self.assertEqual(get_args(Self | int), (Self, int)) | ||
| self.assertEqual(get_args(int | Self), (int, Self)) |
There was a problem hiding this comment.
I think in this case, yes, it's generally important that | works for all these special forms on 3.10+, so it's probably good to explicitly test all of them
Add tests to exercise some previously unexercised lines:
_SpecialGenericAlias.__setattr__()for non-dunder, non-"allowed" attributes (src)_SpecialGenericAliasfrom without defaults (src)_SpecialGenericAliaswith 0 type parameters (src)<thing>.__module__on Python implementations that don't supplysys._getframemodulenameorsys._getframe(src)get_type_hints()withtypes.GenericAlias(src)__init_subclass__()ofParamSpec(src)__or__()and__ror__()of_SpecialForm(src)__repr__and__reduce__ofNoExtraItems(test case copied fromNoDefaultTests) (src)