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

version 4.0 #476

Merged
merged 70 commits into from Jan 29, 2020
Merged

version 4.0 #476

merged 70 commits into from Jan 29, 2020

Conversation

cjw296
Copy link
Collaborator

@cjw296 cjw296 commented Jan 13, 2020

No description provided.

@cjw296

This comment has been minimized.

try:
from asyncio import run
except ImportError:
def run(main):
Copy link
Contributor

Choose a reason for hiding this comment

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

Returning the value will help in fixing some of the cases :

def run(main):
    loop = asyncio.new_event_loop()
    return_value = loop.run_until_complete(main)
    loop.close()
    return return_value

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep, that got them all!

@tirkarthi
Copy link
Contributor

Questions :

  • May I know the commit from upstream used for backporting?
  • Are warnings enabled in CI? Please also enable them if not done to see if there are any warnings regarding mock not awaited that were subtle during upstream AsyncMock review.

Thank you very much Chris for this effort :)

@cjw296
Copy link
Collaborator Author

cjw296 commented Jan 14, 2020

I've started from the revision in ce5b696, it's also stored in lastsync.txt in the repo (the backporting instructions in the docs and backport.py give the full details).

The reason for this is that it's the revision before the problematic positional-only argument commit. As we discussed by email, my plan is to skip that patch and the use backport.py to bring the repo up to cpython's current master before doing a 4.0 release. So, all the changes you mention will get pulled in, along with their problems and challenges :-/

Only DeprecationWarnings are hidden: https://github.com/testing-cabal/mock/blob/master/setup.cfg#L54 - so all others should be visible in the test run output.

I'd suggest waiting until I've got up to current cpython master before really looking at this, if I hit issues before then I'll ping you on this issue for help :-)

@tirkarthi
Copy link
Contributor

Thanks for the details. Sure, I will be happy to help :)

@cjw296
Copy link
Collaborator Author

cjw296 commented Jan 15, 2020

It's interesting to see what's missing coverage:
https://app.circleci.com/jobs/github/testing-cabal/mock/4555
I wish we could introduce coverage gating upstream, especially for a library like mock, there's no excuse for having lines of code that aren't covered by tests.

@cjw296
Copy link
Collaborator Author

cjw296 commented Jan 15, 2020

@lisroach - can you talk me through the change in python/cpython@f1a297a as it causes the following failure on Py3.6 when backported:

______________________________________ SpecSignatureTest.test_autospec_on_bound_builtin_function _______________________________________

self = <mock.tests.testhelpers.SpecSignatureTest testMethod=test_autospec_on_bound_builtin_function>

    @pytest.mark.skipif(IS_PYPY,
                        reason="https://bitbucket.org/pypy/pypy/issues/3010")
    def test_autospec_on_bound_builtin_function(self):
        meth = types.MethodType(time.ctime, time.time())
        self.assertIsInstance(meth(), str)
>       mocked = create_autospec(meth)

mock/tests/testhelpers.py:899: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
mock/mock.py:2603: in create_autospec
    name=_name, **_kwargs)
mock/mock.py:416: in __new__
    if _is_async_obj(bound_args[spec_arg[0]]):
mock/mock.py:52: in _is_async_obj
    return asyncio.iscoroutinefunction(obj) or inspect.isawaitable(obj)
/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/asyncio/coroutines.py:256: in iscoroutinefunction
    _inspect_iscoroutinefunction(func))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

object = <bound method ctime of 1579078759.885324>

    def iscoroutinefunction(object):
        """Return true if the object is a coroutine function.
    
        Coroutine functions are defined with "async def" syntax.
        """
        return bool((isfunction(object) or ismethod(object)) and
>                   object.__code__.co_flags & CO_COROUTINE)
E       AttributeError: 'builtin_function_or_method' object has no attribute '__code__'

/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/inspect.py:185: AttributeError

@tirkarthi
Copy link
Contributor

My upstream PR helps with code coverage for AsyncMock python/cpython#17906 .. IIRC you have pruned some untested code to get to 100% coverage earlier.

@lisroach
Copy link
Contributor

@cjw296 thanks for doing all of this work!

I think that test failure is because the inspect.iscoroutinefunction was fixed between 3.6 and 3.8, in 3.8 it actually checks there is a __code__ before doing the call (python/cpython@fcef60f) to prevent crashing. I made that change in my commit because I found not all Awaitables have a __code__ attribute, making the check invalid in those cases.

We can update upstream to be backwards compatible here, maybe we need to pull in the _has_code_flag function from 3.8's inspect module into mock? That's not very elegant but I'm trying to think of another way around it.

@cjw296
Copy link
Collaborator Author

cjw296 commented Jan 21, 2020

@lisroach - Yeah, for now I've backported the stuff from Python 3.8 in 0c4ceaa. It's bigger than I'd like and maybe we can find a different way once I catch this branch up to cpython master.

mock/mock.py Outdated Show resolved Hide resolved
mock/tests/testmock.py Show resolved Hide resolved
tirkarthi and others added 19 commits January 29, 2020 19:12
…mock (GH-16784)

Backports: 66b00a9d3aacf6ed49412f48743e4913104a2bb3
Signed-off-by: Chris Withers <chris@withers.org>
…H-18116)

Backports: 62865f4532094017a9b780b704686ca9734bc329
Signed-off-by: Chris Withers <chris@withers.org>
…(GH-15521)

Backports: 40c080934b3d49311209b1cb690c2ea1e04df7e7
Signed-off-by: Chris Withers <chris@withers.org>
Backports: aef7dc89879d099dc704bd8037b8a7686fb72838
Signed-off-by: Chris Withers <chris@withers.org>
…gicMock (#16029)

Backports: 72b1004657e60c900e4cd031b2635b587f4b280e
Signed-off-by: Chris Withers <chris@withers.org>
- The gc.collect is needed for other implementations, such as pypy
- Using context managers over multiple lines will only catch the warning from the first line in the context!
- remove a skip for a test that no longer fails on pypy

Backports: a46575a8f2ded8b49e26c25bb67192e1500e76ca
Signed-off-by: Chris Withers <chris@withers.org>
Replace check for whether something is a method in the mock module. The
previous version fails on PyPy, because there no method wrappers exist
(everything looks like a regular Python-defined function). Thus the
isinstance(getattr(result, '__get__', None), MethodWrapperTypes) check
returns True for any descriptor, not just methods.

This condition could also return erroneously True in CPython for
C-defined descriptors.

Instead to decide whether something is a method, just check directly
whether it's a function defined on the class. This passes all tests on
CPython and fixes the bug on PyPy.

Backports: a327677905956ae0b239ff430a1346dfe265709e
Signed-off-by: Chris Withers <chris@withers.org>
* use the `: pass` and `: yield` patterns for code that isn't expected to ever be executed.

* The _Call items passed to _AnyComparer are only ever of length two, so assert instead of if/else

* fix typo

* Fix bug, where stop-without-start patching dict blows up with `TypeError: 'NoneType' object is not iterable`, highlighted by lack of coverage of an except branch.

* The fix for bpo-37972 means _Call.count and _Call.index are no longer needed.

* add coverage for calling next() on a mock_open with readline.return_value set.

* __aiter__ is defined on the Mock so the one on _AsyncIterator is never called.

Backports: db5e86adbce12350c26e7ffc2c6673369971a2dc
Signed-off-by: Chris Withers <chris@withers.org>
…attr__ chaining so that call() can be subscriptable (GH-15565)

* bpo-37972: unittest.mock._Call now passes on __getitem__ to the __getattr__ chaining so that call() can be subscriptable

* 📜🤖 Added by blurb_it.

* Update 2019-08-28-21-40-12.bpo-37972.kP-n4L.rst

added name of the contributor

* bpo-37972: made all dunder methods chainable for _Call

* bpo-37972: delegate only attributes of tuple instead to __getattr__

Backports: 72c359912d36705a94fca8b63d80451905a14ae4
Signed-off-by: Chris Withers <chris@withers.org>
@cjw296
Copy link
Collaborator Author

cjw296 commented Jan 29, 2020

Yep, screwed up backporting on 72c359912d36705a94fca8b63d80451905a14ae4 / GH-15565 so redoing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet