Mock autospec not used, or not used properly

[Claudiu Belu]

Description
===========

In typical unit tests, almost all of the dependencies are mocked or patched (mock.patch), without any guarantee that the mocked methods actually exist, or if their signatures are respected (see below). Because of this, actual issues can easily be overlooked and missed, as the unit tests are wrongfully passing. [0]

The mock.Mock class accepts a spec as an argument, which only solves half the problem: it only checks if an attribute exists, based on the given spec. It does not guarantee that the given attribute is actually a method, or if its signature is respected [1][2]. Some unit tests may pass the autospec argument, but mock doesn't support it at the moment.

mock.patch, mock.patch.object, mock.patch.multiple accept an autospec argument, but because of a bug [3][4], it cannot be used properly.

Steps to reproduce
==================

import mock
from mock.tests import testmock

m = mock.Mock(spec=testmock.Something)

# meth has the following signature:
# def meth(self, a, b, c, d=None):
m.meth()

Expected result
===============

TypeError should be raised.

Actual result
=============

A mock object is returned.

Proposal
========

Bug reports have been issues for the bugs mentioned above, see [1][2][3][4], but until then, the mock library can be patched to include the following changes:

- add autospec argument to mock.Mock and mock.MagicMock. Unit test should then pass the autospec argument whenever possible.
- fix the mock.patch autospec issue.
- enable mock.patch autospec by default, unless otherwise specified.

Links
=====

[0] https://review.openstack.org/#/c/461689/
[1] https://github.com/testing-cabal/mock/issues/393
[2] https://bugs.python.org/issue30587
[3] https://github.com/testing-cabal/mock/issues/396
[4] https://bugs.python.org/issue32092

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/524438

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/524438
Committed: https://git.openstack.org/cgit/openstack/oslotest/commit/?id=0bde35899a10875891f0592961c01e9fbb854738
Submitter: Zuul
Branch: master

commit 0bde35899a10875891f0592961c01e9fbb854738
Author: Claudiu Belu <email address hidden>
Date: Fri Dec 1 03:24:14 2017 +0200

    Adds mock autospec fixture

    In typical unit tests, almost all of the dependencies are mocked or
    patched (mock.patch), without any guarantee that the mocked methods
    actually exist, or if their signatures are respected (see below). Because
    of this, actual issues can easily be overlooked and missed, as the unit
    tests are wrongfully passing.

    The mock.Mock class accepts a spec as an argument, which only solves half
    the problem: it only checks if an attribute exists, based on the given
    spec. It does not guarantee that the given attribute is actually a method,
    or if its signature is respected. Some unit tests may pass the autospec
    argument, but mock doesn't support it at the moment.

    mock.patch, mock.patch.object, mock.patch.multiple accept an autospec
    argument, but because of a bug, it cannot be used properly.

    Adds a fixture which replaces mock.Mock and mock.MagicMock with
    subclass which accepts the autospec argument, and on call, it will
    check the signature of the called method / function.

    Adds a function which replaces mock.mock._patch with a subclass, which
    treats the autospec argument properly (consumes the self / cls argument),
    and sets autospec=True by default, unless otherwise specified.
    WARNING: this function is not a fixture, and in order to benefit from it,
    it will have to be called as EARLY as possible, before any test classes
    are loaded, otherwise the original mock.mock._patch is used instead.

    Needed-By: I3636833962c905faa0f144c7fdc4833037324d31
    Needed-By: I4484e63c97bd1cdde3d88855eabe7545784f365e

    Closes-Bug: #1735588

    Change-Id: I0e4a55fbf4c1d175726ca22b664e240849a99856

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/oslotest 3.2.0 release.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/532947

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/533230

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/533230
Committed: https://git.openstack.org/cgit/openstack/oslotest/commit/?id=5d3196781bc3b51eee04f4ed7a77af0c9fdbcaba
Submitter: Zuul
Branch: master

commit 5d3196781bc3b51eee04f4ed7a77af0c9fdbcaba
Author: Claudiu Belu <email address hidden>
Date: Tue Jan 9 21:46:45 2018 -0800

    mock: Fixes mock.patch.multiple autospec

    mock.patch.multiple causes the patch result to be a dictionary,
    which causes the autospec to fail. This patch will autospec
    properly in this scenario as well.

    Change-Id: I3aa97459bf53059422c8fa696cb09df811c1701b
    Related-Bug: #1735588

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/535806

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/532619
Committed: https://git.openstack.org/cgit/openstack/os-win/commit/?id=823b162c6a02083f6ae6f6ce27150c039687993d
Submitter: Zuul
Branch: master

commit 823b162c6a02083f6ae6f6ce27150c039687993d
Author: Claudiu Belu <email address hidden>
Date: Wed Jan 10 21:47:21 2018 +0200

    tests: Use mock autospec in unit tests

    Autospecing will ensure that the method signatures are respected
    during calls.

    oslotest.mock_fixture contains 2 components, one of them adds the autospec
    argument to mock.Mock and mock.MagicMock, while the other one fixes the
    autospec behaviour for mock.patch functions, and sets autospec to True by
    default, unless otherwise specified.

    Closes-Bug: #1735588

    Change-Id: I5a1dd8571988859b4a14a505fd5e016079582363

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/532947
Committed: https://git.openstack.org/cgit/openstack/networking-hyperv/commit/?id=d5b72f4250ce3a60fb85f751c99fbba88a71ced0
Submitter: Zuul
Branch: master

commit d5b72f4250ce3a60fb85f751c99fbba88a71ced0
Author: Claudiu Belu <email address hidden>
Date: Tue Jan 9 05:53:42 2018 -0800

    tests: Use mock autospec in unit tests

    Autospecing will ensure that the method signatures are respected
    during calls.

    oslotest.mock_fixture contains 2 components, one of them adds the autospec
    argument to mock.Mock and mock.MagicMock, while the other one fixes the
    autospec behaviour for mock.patch functions, and sets autospec to True by
    default, unless otherwise specified.

    Change-Id: Ib785be16e2b81a32dff32b0e070e34e0675d4b4f
    Closes-Bug: #1735588

[OpenStack Infra (hudson-openstack)]

Change abandoned by Matthew Edmonds (<email address hidden>) on branch: master
Review: https://review.openstack.org/535806
Reason: abandoning in favor of https://review.openstack.org/#/c/447505

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/538022

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/networking-hyperv 6.0.0 release.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/539908
Committed: https://git.openstack.org/cgit/openstack/compute-hyperv/commit/?id=26c91401ee8f203d74261a6b0c67196173ef57d6
Submitter: Zuul
Branch: master

commit 26c91401ee8f203d74261a6b0c67196173ef57d6
Author: Claudiu Belu <email address hidden>
Date: Tue Jan 30 03:48:37 2018 -0800

    tests: autospecs mock.patch and classes usages

    By default, mock.patch's autospec argument is None, meaning that
    there's no signature checking for the patched methods and funtions.

    oslotest.mock_fixture.patch_mock_module fixes a few issues within
    mock.patch functions, as well as setting autospec=True by default,
    unless otherwise specified or new_callable, create, spec arguments
    are passed in.

    Updates HyperVBaseTestCase to autospec the classes used by different
    *Ops classes.

    Change-Id: Ifbdc185d0b50e29c5d5f3ff036141253d2323076
    Partial-Bug: #1735588

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/os-win 4.0.0 release.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/538022
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=9c7d6123e3e66fd8b182af548273df9e7d66ee36
Submitter: Zuul
Branch: master

commit 9c7d6123e3e66fd8b182af548273df9e7d66ee36
Author: Claudiu Belu <email address hidden>
Date: Wed Jan 24 23:30:37 2018 -0800

    tests: refactors and cleans up test_rbd.py

    In the unit tests' setUp, it was setting self.mock_rbd = mock_rbd
    (where mock_rbd is the argument passed by @mock.patch.object) and
    setting all sorts of properties onto self.mock_rbd, but mock_rbd
    would become invalid after setUp finished, as the mock.patch
    decorator would remove the patched mock. The same applies with
    mock_rados.

    This patch addresses this issue by refactoring and simplifying the
    unit tests.

    Change-Id: Ie2bf4d559e5391be3fe778835673a11de7ec8e1b
    Related-Bug: #1735588

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/539954
Committed: https://git.openstack.org/cgit/openstack/compute-hyperv/commit/?id=d19606a82809f2a909d20b102166880b65d74b1f
Submitter: Zuul
Branch: master

commit d19606a82809f2a909d20b102166880b65d74b1f
Author: Claudiu Belu <email address hidden>
Date: Tue Jan 30 06:06:47 2018 -0800

    autospecs os-win utils classes

    Most of the os-win utils are created through
    os_win.utilsfactory, which all rely on its function
    '_get_class' to load and initialize the proper Utils
    based on the OS version.

    This commit patches the mention method, returning
    an autospeced mock for the desired Utils instead.

    Closes-Bug: #1735588

    Change-Id: If0002b2c711aaeaf13273bed9b912fdd9c639a64

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/557368

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/447505
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=1f0638f943a7b490db3cf9a60423ebae859682cc
Submitter: Zuul
Branch: master

commit 1f0638f943a7b490db3cf9a60423ebae859682cc
Author: Claudiu Belu <email address hidden>
Date: Mon Mar 20 15:34:37 2017 +0200

    tests: fixes mock autospec usage

    Until now, mock.Mock did not receive any autospec argument, yet
    there are a few tests which are using it.

    The autospec argument is being added by a fixture, and some unit
    tests are not using it properly.

    Depends-On: I0e4a55fbf4c1d175726ca22b664e240849a99856

    Partial-Bug: #1735588

    Change-Id: I3636833962c905faa0f144c7fdc4833037324d31

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/557923

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/557368
Committed: https://git.openstack.org/cgit/openstack/oslotest/commit/?id=bb78b84c3fc8485e590f2c925c07731a9740a852
Submitter: Zuul
Branch: master

commit bb78b84c3fc8485e590f2c925c07731a9740a852
Author: Claudiu Belu <email address hidden>
Date: Wed Mar 28 01:07:18 2018 -0700

    mock: Properly patch mock.MagicMock

    MockAutospecFixture should patch both internal and external usages
    of MagicMock (mock.MagicMock and mock.mock.MagicMock). However, only
    the internal one is patched properly.

    Related-Bug: #1735588

    Change-Id: Ib9709f1cf5dbed4792f5dd7c49d8f9c77f04419f

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/557923
Committed: https://git.openstack.org/cgit/openstack/oslotest/commit/?id=6baff0eda4edbd8163426a2ac774514cabf99a3a
Submitter: Zuul
Branch: master

commit 6baff0eda4edbd8163426a2ac774514cabf99a3a
Author: Claudiu Belu <email address hidden>
Date: Thu Mar 29 04:57:48 2018 -0700

    mock: Apply autospec to a mock's return_value

    When creating a mock with a class as an autospec:

    mock_foo = mock.Mock(autospec=FooClass)

    The autospec is only applied to mock_foo itself, and it will
    behave as expected. However, the autospec is not applied to its
    return_value and thus, the method validation is not enforced:

    mock_foo().lish(some_argument_which_doesnt_exist=42)

    This patch addresses this issue, and adds necessary unit tests to
    test this behaviour.

    Change-Id: Icd96beba5a32001cf33f075b801471c6e7c75898
    Related-Bug: #1735588

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/561581
Committed: https://git.openstack.org/cgit/openstack/oslotest/commit/?id=8241dd624f8ca481fe15a44f7d3fed10be7ae937
Submitter: Zuul
Branch: master

commit 8241dd624f8ca481fe15a44f7d3fed10be7ae937
Author: Claudiu Belu <email address hidden>
Date: Fri Apr 13 10:33:12 2018 -0700

    mock: Perform patch's autospec checks on __enter__

    Currently we're doing the autospec checks on __init__ (if the
    target is autospecable), which will require importing modules
    in some cases.

    The mock.patch's constructor is called when a test module is being
    loaded, before any set up could have been run, which can be problematic
    in some cases (e.g.: some tested modules are importing platform specific
    modules, and they couldn't have been mocked yet).

    This patch moves the autospec checks to __enter__, which is executed
    after the setUp.

    Related-Bug: #1735588

    Change-Id: I9e10b34092ad795c7f9e58596fcccf4f37856225

[OpenStack Infra (hudson-openstack)]

Change abandoned by Claudiu Belu (<email address hidden>) on branch: master
Review: https://review.openstack.org/557974

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/470775
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=1eec451b1ba254ec4c43c77f79b0d2809d6cb9a5
Submitter: Zuul
Branch: master

commit 1eec451b1ba254ec4c43c77f79b0d2809d6cb9a5
Author: Takashi NATSUME <email address hidden>
Date: Mon Aug 19 18:11:48 2019 +0900

    Tests: autospecs all the mock.patch usages

    By default, mock.patch's autospec argument is None, meaning that
    there's no signature checking for the patched methods and functions.

    oslotest.mock_fixture.patch_mock_module fixes a few issues within
    mock.patch functions, as well as setting autospec=True by default,
    unless otherwise specified or new_callable, create, spec arguments
    are passed in.

    Co-Authored-By: Claudiu Belu <email address hidden>
    Change-Id: I4484e63c97bd1cdde3d88855eabe7545784f365e
    Closes-Bug: #1735588

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/compute-hyperv 9.0.0.0rc1 release candidate.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/nova 20.0.0.0rc1 release candidate.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Matt Riedemann (<email address hidden>) on branch: master
Review: https://review.opendev.org/342211
Reason: This looks dead and I'm not sure if it's still needed given I4484e63c97bd1cdde3d88855eabe7545784f365e landed.