Skip to content

typing tests: remove some unnecessary uses of exec() #119005

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.

Sign up for GitHub

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

Merged
JelleZijlstra merged 2 commits into from
May 14, 2024
Merged

typing tests: remove some unnecessary uses of exec() #119005

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
168d87b
typing tests: remove some unnecessary uses of `exec()`
AlexWaygood May 13, 2024
6f25644
address review
AlexWaygood May 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 10 additions & 19 deletions Lib/test/test_typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7060,24 +7060,16 @@ def test_iterator(self):
self.assertNotIsInstance(42, typing.Iterator)

def test_awaitable(self):
ns = {}
exec(
"async def foo() -> typing.Awaitable[int]:\n"
" return await AwaitableWrapper(42)\n",
globals(), ns)
foo = ns['foo']
async def foo() -> typing.Awaitable[int]:
return await AwaitableWrapper(42)
g = foo()
self.assertIsInstance(g, typing.Awaitable)
self.assertNotIsInstance(foo, typing.Awaitable)
g.send(None) # Run foo() till completion, to avoid warning.

def test_coroutine(self):
ns = {}
exec(
"async def foo():\n"
" return\n",
globals(), ns)
foo = ns['foo']
async def foo():
return
g = foo()
self.assertIsInstance(g, typing.Coroutine)
with self.assertRaises(TypeError):
Expand Down Expand Up @@ -7352,10 +7344,9 @@ def test_no_generator_instantiation(self):
typing.Generator[int, int, int]()

def test_async_generator(self):
ns = {}
exec("async def f():\n"
" yield 42\n", globals(), ns)
g = ns['f']()
async def f():
yield 42
g = f()
self.assertIsSubclass(type(g), typing.AsyncGenerator)

def test_no_async_generator_instantiation(self):
Expand Down Expand Up @@ -7442,9 +7433,9 @@ def asend(self, value):
def athrow(self, typ, val=None, tb=None):
pass

ns = {}
exec('async def g(): yield 0', globals(), ns)
g = ns['g']
async def g(): yield 0
g = g()
Copy link
Member

@JelleZijlstra JelleZijlstra May 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the old code here g was the function, now it's the coroutine returned from the function.

Copy link
Member Author

@AlexWaygood AlexWaygood May 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch. Sorry, that was sloppy.

Copy link
Member Author

@AlexWaygood AlexWaygood May 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although on second thought... doesn't it make more sense to test the coroutine returned from the function here? I think the test might be buggy as-written.

It passes regardless, and this PR wasn't meant to make any semantic changes to the tests, so I'm happy to leave it as-is for now (I pushed 6f25644), but it looks odd to me

Copy link
Member

@JelleZijlstra JelleZijlstra May 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's odd that the passes both with and without the g(). Might be worth looking into whether we can make the test more effective.

Copy link
Member Author

@AlexWaygood AlexWaygood May 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense that it passes both with and without the g(), because g is only used to check that the [coroutine-function or coroutine] is not an instance of the custom AsyncGenerator subclass here:

cpython/Lib/test/test_typing.py

Lines 7459 to 7460 in 7d7eec5

self.assertNotIsInstance(type(g), G)
self.assertNotIsInstance(g, G)

And of course, it is indeed true that neither a coroutine nor a coroutine function will be an instance of the custom AsyncGenerator subclass


self.assertIsSubclass(G, typing.AsyncGenerator)
self.assertIsSubclass(G, typing.AsyncIterable)
self.assertIsSubclass(G, collections.abc.AsyncGenerator)
Expand Down