gh-76273: Fix autospec mocking of instance and class methods

[stephenfin]

Currently, when patching instance / class methods with autospec, their self / cls arguments are not consumed, causing call asserts to fail (they expect an instance / class reference as the first argument).
Example:

from unittest import mock

class Something(object):
    def foo(self, a, b, c, d):
        pass

with mock.patch.object(Something, 'foo', autospec=True):
    s = Something()
    s.foo()

Fix this by skipping the first argument when presented with a method.

Based on #4476 with the changes to the Mock signature stripped out and left for a separate, follow-up PR.

• Issue: mock.patch with autospec does not consume self / cls argument #76273

[johnslavik]

Hi! Nice to see some work on this. Btw, we generally avoid force pushes.

[stephenfin]

Btw, we generally avoid force pushes.

Understood. I've got a small bit more work to do this on to resolve the test_hmac_constructor_uses_builtin failure (we need to handle calling patch.object(cls, name, autospec=True, wraps=wraps) where wraps is unbound). If I push a patch will it be squashed on merge? I can't see anything in the contributor docs in relation to this.

[johnslavik]

If I push a patch will it be squashed on merge?

Yes, all PRs are squashed on merge.

I can't see anything in the contributor docs in relation to this.

I see! It's in https://devguide.python.org/getting-started/pull-request-lifecycle/#don-t-force-push. Maybe we can make it more visible elsewhere, what do you think? Let me know what you had read before you created the PR, maybe we can add it there :-) Thanks for saying that you didn't see that note.

[stephenfin]

Just noting that I do plan to get back to this. I just need to find time to do so.

If I push a patch will it be squashed on merge?

Yes, all PRs are squashed on merge.

Ack, thanks 👌

I can't see anything in the contributor docs in relation to this.

I see! It's in devguide.python.org/getting-started/pull-request-lifecycle#don-t-force-push. Maybe we can make it more visible elsewhere, what do you think? Let me know what you had read before you created the PR, maybe we can add it there :-) Thanks for saying that you didn't see that note.

To be fair, I just looked the Makefile and README in Tools. I hadn't gone as far as looking at external docs.

[github-actions]

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

[SeanMooney]

as an aside, i really like using autospec in general but there is a related edge case to what stephen is tryign to fix here that has bit me when backporting chagnes to branches that still support python 3.9

def _writer(fn):
    # oslo.db uses functools.wraps which sets __wrapped__. Python 3.9's
    # mock.autospec follows __wrapped__ and then fails to strip `self` for
    # the bound-method case, shifting positional args by one. Removing
    # __wrapped__ makes autospec see (*args, **kwargs) instead.
    wrapped = main_context_manager.writer(fn)
    del wrapped.__wrapped__
    return wrapped


def _reader(fn):
    wrapped = main_context_manager.reader(fn)
    del wrapped.__wrapped__
    return wrapped 

on 3.9 i had to do that hack to make functions that had multiple layers of decorators

    @oslo_db_api.retry_on_deadlock
    @_writer
    def _do_update_device(self, context, uuid, values):
        query = model_query(context, models.Device)
        query = add_identity_filter(query, uuid)
        try:
            ref = query.with_for_update().one()
        except NoResultFound:
            raise exception.ResourceNotFound(
                resource='Device',
                msg='with uuid=%s' % uuid)

        ref.update(values)
        return ref

actully work properly with autospec in a version independent way.

so it would be good to test with a function that has 2+ levels fo decortors that internally use functools.wraps to ensure that we do not regress the 3.10 behvior adn that the self/cls parmater are properly handeled