magic handling of mock.patch() is confusing and not generic enough.

[cjw296]

Okay, so I'm raising this as a result of:

https://github.com/Simplistix/testfixtures/issues/65

testfixtures' log_capture() (and a couple of other decorators it offers) use the same pattern as mock.patch as implemented here:

https://github.com/Simplistix/testfixtures/blob/efc8082211e1c462aea23307d4ff4788e582bdce/testfixtures/utils.py#L24

However, I see that mock.patch only works because of the magical handling here:

https://github.com/pytest-dev/pytest/blob/46e30435ebad1796e54d99572d43a5b6a1961a19/_pytest/compat.py#L74

This means that end users and library developers are confused when one works and not the other does not. Not sure I have any good suggestions, just wanted to vent over the hours I've wasted on this...

[RonnyPfannschmidt]

the correct way forward is to find a signature and use it - #2267 should be a way forward then the other decorators just have to fix the signature

[cjw296]

How will inspect.signature know that mock.patch or testfixtures.log_capture intend to fill in one or more of the arguments?

The wrappers just take (*args, **kw) and going through __wrapped__ (which doesn't exist in Py2?) means you just end up trying to fill in with fixtures arguments that will be provided by the wrapper.

My suggestion would be to allow @pytest.mark.usefixtures("...") to be used to explicitly override and say what fixtures should be provided, and then provide those as keyword arguments to keep out of the way.

This approach has worked really well for me with another dependency injection project here, where implicit dependency injection based on argument name is only used as a fallback:

http://mush.readthedocs.io/en/latest/use.html#declarative-configuration http://mush.readthedocs.io/en/latest/use.html#default-configuration

[RonnyPfannschmidt]

the correct way for functions to change a signature is to set a new one, now mock doesn't do that already, and im inclined to go ahead and start to warn about this so that all tests that are detected as wrapepers will be warned about

id much prefer direct injection in all cases (aka involve pytest machiery) as well as getting the fixture mechanism to be more a library and less a pytest only hell that cant be reused

[cjw296]

How does a library set a new signature in a graceful way that supports both Py2 and Py3?

I'd prefer no more warnings that end users who will see them can't easily do anything about...

Not sure what you mean by 'direct injection in all cases'?

I'm planning to pivot mush somewhat such that it's primary use will be:

from mush import Context, requires

context = Context()

def my_func(the_thing):
    ...

context.add(SomeThing(), returns='the_thing')
context.call(my_func)

# or:
context.call(my_func, requires='the_thing')

# or instead define the function as one of the following:

@requires('the_thing')
def my_func(another_name):
    ...

def my_func(another_name: 'the_thing'):
    ...

def my_func(another_name: SomeThing):
    ...

Would that be of use/interest?

[RonnyPfannschmidt]

@cjw296 wrt setting signatures for all python version, use funcsigs, pytest will, too

wrt dependency injection in pytest - the injector needs to support life-cycle management and nesting of livecycles, so the mush example cant help us,

at the backend we have scopes and scoped parameters as dependency and would like to control tear-down of them and their dependencies, we should probably open a new issue for that discussion

[cjw296]

How do you use funcsigs to set a new signature? (hopefully that's a silly question with an easy answer!)

For the dependency injection, yes, either ping me an email or open a new ticket and CC me in. I suspect Mush already does a bunch of the needed stuff (providing a registry of dependencies and a nice interface for looking them up, layering lifecycles and scoping over that may be something that can sensibly be added, or could just be something that lives elsewhere but uses the stuff that Mush does do...)

[RonnyPfannschmidt]

@cjw296 signatures are set on a wrapper by adding a signature object

[nicoddemus]

Is this fixed by #2842?

[RonnyPfannschmidt]

@nicoddemus no, because the libraries in question also must use the mechanism to annotate

[cjw296]

@nicoddemus @RonnyPfannschmidt - what is the mechanism in question? I'd happily do the necessary in testfixtures, just as long as it's not a special case hack for pytest.

[RonnyPfannschmidt]

@cjw296 the required mechanism needs to set a __signature__ on the wrapper object that has only the correct data - basically any lib not mock can do that to get correct data

however mock is broken in many python versions and part of the stdlib, so we will have to deal with its mess forever

[cjw296]

Is __signature__ an "official python thing" now? If so, where are the docs? Happy to use it if it's official ;-)

ps: you at PyConUK?

[RonnyPfannschmidt]

https://www.python.org/dev/peps/pep-0362/ - in particular https://www.python.org/dev/peps/pep-0362/#visualizing-callable-objects-signature

im not at PyConUK - i wont be at conferences until the kid is able to walk

[cjw296]

Ah cool, I'm more than happy to do that for the testfixtures stuff...

[nicoddemus]

We are following the PEP already and testfixtures will be gracefully updated by @cjw296, so I guess there's nothing else for pytest to do here. Should we close this then?

[cjw296]

@nicoddemus - sorry, just to confirm: current releases of pytest will respect a __signature__ on both Python 2 and Python 3? What should I use to generate __signature__ on Python 2?

[nicoddemus]

AFAIU yes, by using funcsigs in Python 2 we are now interpreting __signature__ correctly in all platforms. @RonnyPfannschmidt can you confirm this?

[nicoddemus]

Oh and that will be available in pytest 3.3+, which we plan to release in the next couple of weeks.

[cjw296]

Can we leave this open until 3.3 is released? I hope you're duck typing whatever's in __signature__? I want to try and avoid having funcsigs as a dependency if possible...

[RonnyPfannschmidt]

we use funcsigs to create/manipulate the objects and if you want to implement the same api, good luck with the minor details across python versions

[RonnyPfannschmidt]

based on https://github.com/pytest-dev/pytest/pull/2842/files you will face massive trouble if you dont match the constants

[cjw296]

Yeah, unless your comments about __wrapped__ and getting constants through the instance are addressed, this is a non-starter for me...

[RonnyPfannschmidt]

@cjw296 __wrapped__ should be irrelevant for you, but you ought to take some kind of care that we can find that messy mock metadata where we expect it in order to cut out the mock things

the constants need to be addressed - i#ll make a critical issue blocking the release

[nicoddemus]

Is this still an issue? It is marked as "Need information", what information is missing?

[cjw296]

I think from a pytest perspective, this can be closed.