Skip to content

bpo-44730: Add handling of generator functions #27315

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

Closed
wants to merge 6 commits into from
Closed

bpo-44730: Add handling of generator functions #27315

wants to merge 6 commits into from

Conversation

gmjw
Copy link

@gmjw gmjw commented Jul 24, 2021

Previously generator functions were changed by patches applied as decorators, ending up as non-generator functions (co_flags changed from 99 to 31). Patches were not applied properly in these generator functions.

This PR addresses this by adding a simple switch in mock.py to handle generator functions cleanly.

https://bugs.python.org/issue44730

Previously generator functions were changed by patches applied as decorators, ending up as non-generator functions (co_flags changed from 31 to 99). Patches were not applied properly in these generator functions.

Making the change here seems to address this.
@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@gmjw

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@gmjw
Copy link
Author

gmjw commented Jul 24, 2021

I would be happy to flesh these changes out with a test or two, of course - there is an example associated with the bug report which could easily be made into a simple unit test.

I've never contributed to the repo before so I'll await some general feedback before proceeding with anything.

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Aug 24, 2021
with self.decoration_helper(patched,
args,
keywargs) as (newargs, newkeywargs):
yield from func(*newargs, **newkeywargs)
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't preserve the original characteristics. For example on the following function

def gen_1():
    yield 1
    return 2

this will simply raise StopIteration(None) instead of StopIteration(2) at the end. You should return the result of the generator (see PEP 380)

Copy link
Author

Choose a reason for hiding this comment

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

You're absolutely right. Thanks, I forgot that generator functions could also return values.

@MaxwellDupre
Copy link
Contributor

I can't get this to run correctly:

./python -m unittest Lib/unittest/test/testmock/testpatch.py
Traceback (most recent call last):
File "/home/me/Documents/cpython/Lib/importlib/init.py", line 16, in
import _frozen_importlib as _bootstrap
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ModuleNotFoundError: import of _frozen_importlib halted; None in sys.modules

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "", line 198, in _run_module_as_main
File "", line 88, in _run_code
File "/home/me/Documents/cpython/Lib/unittest/main.py", line 18, in
main(module=None)
^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/unittest/main.py", line 100, in init
self.parseArgs(argv)
^^^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/unittest/main.py", line 147, in parseArgs
self.createTests()
^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/unittest/main.py", line 158, in createTests
self.test = self.testLoader.loadTestsFromNames(self.testNames,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/unittest/loader.py", line 198, in loadTestsFromNames
suites = [self.loadTestsFromName(name, module) for name in names]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/unittest/loader.py", line 198, in
suites = [self.loadTestsFromName(name, module) for name in names]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/unittest/loader.py", line 132, in loadTestsFromName
module = import(module_name)
^^^^^^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/unittest/test/testmock/testpatch.py", line 13, in
from test.test_importlib.util import uncache
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/test/test_importlib/util.py", line 96, in
source_importlib = import_importlib('importlib')['Source']
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/test/test_importlib/util.py", line 52, in import_importlib
source = import_helper.import_fresh_module(module_name, fresh=fresh,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/contextlib.py", line 153, in exit
self.gen.throw(typ, value, traceback)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/test/support/import_helper.py", line 26, in _ignore_deprecated_imports
yield
^^^^^
File "/home/me/Documents/cpython/Lib/contextlib.py", line 153, in exit
self.gen.throw(typ, value, traceback)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/test/support/import_helper.py", line 102, in frozen_modules
yield
^^^^^
File "/home/me/Documents/cpython/Lib/test/support/import_helper.py", line 159, in import_fresh_module
return importlib.import_module(name)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/importlib/init.py", line 126, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/importlib/init.py", line 19, in
_bootstrap._setup(sys, _imp)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/importlib/_bootstrap.py", line 1247, in _setup
loader._setup_module(module)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/importlib/_bootstrap.py", line 829, in _setup_module
assert not hasattr(module, 'file'), module.file
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: /home/me/Documents/cpython/Lib/zipimport.py

cpython on  gmjw-patch-generator-functions-1 [$?] via 🐍 v3.11.0a6+

How to you test testpatch?

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 11, 2022
@gmjw gmjw closed this by deleting the head repository Mar 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants