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

Can no longer patch flask.g #84982

Closed
RobTaft mannequin opened this issue May 28, 2020 · 9 comments
Closed

Can no longer patch flask.g #84982

RobTaft mannequin opened this issue May 28, 2020 · 9 comments
Labels
3.8 (EOL) end of life 3.9 only security fixes tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@RobTaft
Copy link
Mannequin

RobTaft mannequin commented May 28, 2020

BPO 40805
Nosy @terryjreedy, @cjw296, @tirkarthi, @rkm

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-06-08.13:38:19.632>
created_at = <Date 2020-05-28.12:19:57.115>
labels = ['3.8', 'type-bug', 'tests', '3.9']
title = 'Can no longer patch flask.g'
updated_at = <Date 2020-06-08.13:38:19.631>
user = 'https://bugs.python.org/RobTaft'

bugs.python.org fields:

activity = <Date 2020-06-08.13:38:19.631>
actor = 'Rob Taft'
assignee = 'none'
closed = True
closed_date = <Date 2020-06-08.13:38:19.632>
closer = 'Rob Taft'
components = ['Tests']
creation = <Date 2020-05-28.12:19:57.115>
creator = 'Rob Taft'
dependencies = []
files = []
hgrepos = []
issue_num = 40805
keywords = []
message_count = 9.0
messages = ['370197', '370356', '370368', '370650', '370651', '370673', '370675', '370678', '370981']
nosy_count = 5.0
nosy_names = ['terry.reedy', 'cjw296', 'xtreak', 'rkm', 'Rob Taft']
pr_nums = []
priority = 'normal'
resolution = 'third party'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue40805'
versions = ['Python 3.8', 'Python 3.9']

@RobTaft
Copy link
Mannequin Author

RobTaft mannequin commented May 28, 2020

Whenever I try to patch flask.g, it appears to do nothing. This happened when I upgraded mock from 3.x to 4.x. I reported it on the mock github page testing-cabal/mock#490 and was asked to report it here. The folllowing code run with pytest works fine in mock 3.0.5, but fails to patch in 4.0.0 and up.

from mock import patch

import flask


def some_function():
    flask.g.somevariable = True
    return flask.g.somevariable

@patch('flask.g')
def test_some_function(mock_flask_global):
assert some_function()

@RobTaft RobTaft mannequin added 3.8 (EOL) end of life tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error labels May 28, 2020
@terryjreedy
Copy link
Member

Sorry, but I believe you were misdirected*. mock, as opposed to unittest.mock, pytest, flask, and werkzeug are 3rd party modules. The error report seems to be missing part of the stacktrace at both ends. What line is your file resulted in the error? (This time we can guess == @patch. @what called _lookup_app_object in flask? (I have no idea.)

However, once contextlib._GeneratorContextManager calls next(self.gen), the rest of the trace is outside the stdlib. At the end, the RuntimeError is raised by flask, not by python, because the flask _app_ctx_stack.top does not exist. If you do not understand their error message, ask flask people.

  • I assume that cjw296 *glanced* at your report, saw 'RuntimeError', and too quickly assumed 'cpython error'. flask could have defined, for instance, FlaskRuntimeError for its error reporting.

@RobTaft
Copy link
Mannequin Author

RobTaft mannequin commented May 30, 2020

The test was supposed to patch the flask component during the unit test,
the error indicates the patch did not work. The actual error message is
not relevant to the actual issue. I don't know why I was directed to here.
When I replace it with unittest.mock, it appears to work fine, so my
solution might end up being to dump the 3rd party mock library.

On Fri, May 29, 2020 at 11:02 PM Terry J. Reedy <[email protected]>
wrote:

Terry J. Reedy <[email protected]> added the comment:

Sorry, but I believe you were misdirected*. mock, as opposed to
unittest.mock, pytest, flask, and werkzeug are 3rd party modules. The
error report seems to be missing part of the stacktrace at both ends. What
line is your file resulted in the error? (This time we can guess ==
@patch. @what called _lookup_app_object in flask? (I have no idea.)

However, once contextlib._GeneratorContextManager calls next(self.gen),
the rest of the trace is outside the stdlib. At the end, the RuntimeError
is raised by flask, not by python, because the flask _app_ctx_stack.top
does not exist. If you do not understand their error message, ask flask
people.

  • I assume that cjw296 *glanced* at your report, saw 'RuntimeError', and
    too quickly assumed 'cpython error'. flask could have defined, for
    instance, FlaskRuntimeError for its error reporting.

----------
nosy: +terry.reedy
resolution: -> third party
stage: -> resolved
status: open -> closed


Python tracker <[email protected]>
<https://bugs.python.org/issue40805\>


@cjw296
Copy link
Contributor

cjw296 commented Jun 3, 2020

Terry, mock is now a rolling backport of unittest.mock with all development taking place in cpython's repo. If issues are reported there, they need to be triaged here first, as it's most likely a bug in unittest.mock that needs fixing here.

Rob, please can you see if this is reproducible on Python 3.8 or 3.9, replacing from mock import with from unittest.mock import?

@cjw296 cjw296 reopened this Jun 3, 2020
@cjw296 cjw296 reopened this Jun 3, 2020
@cjw296
Copy link
Contributor

cjw296 commented Jun 3, 2020

Rob, you're welcome to dump mock and use unittest.mock, and that might be best for now, but this will then likely come back to bite you when you end up on the version of Python, probably 3.9, where it is present ;-)

@RobTaft
Copy link
Mannequin Author

RobTaft mannequin commented Jun 3, 2020

I have confirmed that using unittest.mock instead of the 3rd party mock library in python 3.8.3 and 3.9-dev fails to patch flask.g. 3.7.7 works correctly.

@RobTaft RobTaft mannequin added 3.9 only security fixes labels Jun 3, 2020
@terryjreedy
Copy link
Member

Chris, I see no evidence in the *incomplete* traceback not posted here that there is a bug in mock. Mock calls "hasattr(obj, '__func__') and apparently flask, called via werkzeug, answers 'RuntimeError'. The overt bug is that calling werkzeug.local.(type(obj)?).__getattr__ fails by doing something other than "return the (computed) attribute value or raise an AttributeError exception".
https://docs.python.org/3/reference/datamodel.html#object.\_\_getattr__

@RobTaft
Copy link
Mannequin Author

RobTaft mannequin commented Jun 3, 2020

I updated the test with 2 cases and the traceback is different for each when I expected them to be the same if this was purely a mock issue, the line throwing the error is the same. I can post this over with flask and see what they think.

from unittest.mock import patch

import flask

def some_function():
    flask.g.somevariable = True
    return flask.g.somevariable

@patch('flask.g')
def test_patch(mock_flask_global):
assert some_function()

def test_no_patch():
    assert some_function()
$ pytest -vv temp_test.py 
============================================================================================================ test session starts 

=============================================================================================================
platform linux -- Python 3.8.3, pytest-5.4.3, py-1.8.1, pluggy-0.13.1 --
collected 2 items

temp_test.py::test_patch FAILED [ 50%]
temp_test.py::test_no_patch FAILED [100%]

================================================================================================================== FAILURES ==================================================================================================================
_________________________________________________________________________________________________________________ test_patch _________________________________________________________________________________________________________________

args = (), keywargs = {}

    @wraps(func)
    def patched(*args, **keywargs):
>       with self.decoration_helper(patched,
                                    args,
                                    keywargs) as (newargs, newkeywargs):

../../../.pyenv/versions/3.8.3/lib/python3.8/unittest/mock.py:1322:


../../../.pyenv/versions/3.8.3/lib/python3.8/contextlib.py:113: in __enter__
return next(self.gen)
../../../.pyenv/versions/3.8.3/lib/python3.8/unittest/mock.py:1304: in decoration_helper
arg = exit_stack.enter_context(patching)
../../../.pyenv/versions/3.8.3/lib/python3.8/contextlib.py:425: in enter_context
result = _cm_type.__enter__(cm)
../../../.pyenv/versions/3.8.3/lib/python3.8/unittest/mock.py:1416: in __enter__
if spec is None and _is_async_obj(original):
../../../.pyenv/versions/3.8.3/lib/python3.8/unittest/mock.py:51: in _is_async_obj
if hasattr(obj, '__func__'):
pyvenv/lib/python3.8/site-packages/werkzeug/local.py:347: in __getattr__
return getattr(self._get_current_object(), name)
pyvenv/lib/python3.8/site-packages/werkzeug/local.py:306: in _get_current_object
return self.__local()


name = 'g'

    def _lookup_app_object(name):
        top = _app_ctx_stack.top
        if top is None:
>           raise RuntimeError(_app_ctx_err_msg)
E           RuntimeError: Working outside of application context.
E           
E           This typically means that you attempted to use functionality that needed
E           to interface with the current application object in some way. To solve
E           this, set up an application context with app.app_context().  See the
E           documentation for more information.

pyvenv/lib/python3.8/site-packages/flask/globals.py:45: RuntimeError
_______________________________________________________________________________________________________________ test_no_patch ________________________________________________________________________________________________________________

    def test_no_patch():
>       assert some_function()

temp_test.py:14:


temp_test.py:6: in some_function
flask.g.somevariable = True
pyvenv/lib/python3.8/site-packages/werkzeug/local.py:364: in <lambda>
__setattr__ = lambda x, n, v: setattr(x._get_current_object(), n, v)
pyvenv/lib/python3.8/site-packages/werkzeug/local.py:306: in _get_current_object
return self.__local()


name = 'g'

    def _lookup_app_object(name):
        top = _app_ctx_stack.top
        if top is None:
>           raise RuntimeError(_app_ctx_err_msg)
E           RuntimeError: Working outside of application context.
E           
E           This typically means that you attempted to use functionality that needed
E           to interface with the current application object in some way. To solve
E           this, set up an application context with app.app_context().  See the
E           documentation for more information.

pyvenv/lib/python3.8/site-packages/flask/globals.py:45: RuntimeError

@RobTaft
Copy link
Mannequin Author

RobTaft mannequin commented Jun 8, 2020

pallets/flask#3637

I've worked around the issue and accept that this will not work in the future.

@RobTaft RobTaft mannequin closed this as completed Jun 8, 2020
@RobTaft RobTaft mannequin closed this as completed Jun 8, 2020
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
rbtcollins added a commit to rbtcollins/cpython that referenced this issue May 27, 2024
In order to patch flask.g e.g. as in python#84982, that
proxies getattr must not be invoked. For that,
mock must not try to read from the original
object. In some cases that is unavoidable, e.g.
when doing autospec. However, patch("flask.g",
new_callable=MagicMock) should be entirely safe.
rbtcollins added a commit to rbtcollins/cpython that referenced this issue May 27, 2024
…new_callable is set

In order to patch flask.g e.g. as in python#84982, that
proxies getattr must not be invoked. For that,
mock must not try to read from the original
object. In some cases that is unavoidable, e.g.
when doing autospec. However, patch("flask.g",
new_callable=MagicMock) should be entirely safe.
rbtcollins added a commit to rbtcollins/cpython that referenced this issue May 27, 2024
…new_callable is set

In order to patch flask.g e.g. as in python#84982, that
proxies getattr must not be invoked. For that,
mock must not try to read from the original
object. In some cases that is unavoidable, e.g.
when doing autospec. However, patch("flask.g",
new_callable=MagicMock) should be entirely safe.
rbtcollins added a commit to rbtcollins/cpython that referenced this issue Jun 10, 2024
…new_callable is set

In order to patch flask.g e.g. as in python#84982, that
proxies getattr must not be invoked. For that,
mock must not try to read from the original
object. In some cases that is unavoidable, e.g.
when doing autospec. However, patch("flask.g",
new_callable=MagicMock) should be entirely safe.
cjw296 pushed a commit that referenced this issue Jun 11, 2024
…le is set (#119601)

In order to patch flask.g e.g. as in #84982, that
proxies getattr must not be invoked. For that,
mock must not try to read from the original
object. In some cases that is unavoidable, e.g.
when doing autospec. However, patch("flask.g",
new_callable=MagicMock) should be entirely safe.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 11, 2024
…callable is set (pythonGH-119601)

In order to patch flask.g e.g. as in pythonGH-84982, that
proxies getattr must not be invoked. For that,
mock must not try to read from the original
object. In some cases that is unavoidable, e.g.
when doing autospec. However, patch("flask.g",
new_callable=MagicMock) should be entirely safe.
(cherry picked from commit 422c4fc)

Co-authored-by: Robert Collins <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 11, 2024
…callable is set (pythonGH-119601)

In order to patch flask.g e.g. as in pythonGH-84982, that
proxies getattr must not be invoked. For that,
mock must not try to read from the original
object. In some cases that is unavoidable, e.g.
when doing autospec. However, patch("flask.g",
new_callable=MagicMock) should be entirely safe.
(cherry picked from commit 422c4fc)

Co-authored-by: Robert Collins <[email protected]>
cjw296 pushed a commit that referenced this issue Jun 11, 2024
…_callable is set (GH-119601) (#120335)

gh-119600: mock: do not access attributes of original when new_callable is set (GH-119601)

In order to patch flask.g e.g. as in GH-84982, that
proxies getattr must not be invoked. For that,
mock must not try to read from the original
object. In some cases that is unavoidable, e.g.
when doing autospec. However, patch("flask.g",
new_callable=MagicMock) should be entirely safe.
(cherry picked from commit 422c4fc)

Co-authored-by: Robert Collins <[email protected]>
cjw296 pushed a commit that referenced this issue Jun 11, 2024
…_callable is set (GH-119601) (#120334)

gh-119600: mock: do not access attributes of original when new_callable is set (GH-119601)

In order to patch flask.g e.g. as in GH-84982, that
proxies getattr must not be invoked. For that,
mock must not try to read from the original
object. In some cases that is unavoidable, e.g.
when doing autospec. However, patch("flask.g",
new_callable=MagicMock) should be entirely safe.
(cherry picked from commit 422c4fc)

Co-authored-by: Robert Collins <[email protected]>
mrahtz pushed a commit to mrahtz/cpython that referenced this issue Jun 30, 2024
…callable is set (python#119601)

In order to patch flask.g e.g. as in python#84982, that
proxies getattr must not be invoked. For that,
mock must not try to read from the original
object. In some cases that is unavoidable, e.g.
when doing autospec. However, patch("flask.g",
new_callable=MagicMock) should be entirely safe.
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
…callable is set (python#119601)

In order to patch flask.g e.g. as in python#84982, that
proxies getattr must not be invoked. For that,
mock must not try to read from the original
object. In some cases that is unavoidable, e.g.
when doing autospec. However, patch("flask.g",
new_callable=MagicMock) should be entirely safe.
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
…callable is set (python#119601)

In order to patch flask.g e.g. as in python#84982, that
proxies getattr must not be invoked. For that,
mock must not try to read from the original
object. In some cases that is unavoidable, e.g.
when doing autospec. However, patch("flask.g",
new_callable=MagicMock) should be entirely safe.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.8 (EOL) end of life 3.9 only security fixes tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants