Mock autospec not used, or not used properly
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
OpenStack Compute (nova) |
Fix Released
|
Undecided
|
Takashi Natsume | ||
compute-hyperv |
Fix Released
|
Undecided
|
Unassigned | ||
networking-hyperv |
Fix Released
|
Undecided
|
Claudiu Belu | ||
os-win |
Fix Released
|
Undecided
|
Claudiu Belu | ||
oslotest |
Fix Released
|
Undecided
|
Claudiu Belu |
Bug Description
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(
# 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:/
[1] https:/
[2] https:/
[3] https:/
[4] https:/
Changed in nova: | |
assignee: | nobody → Claudiu Belu (cbelu) |
Changed in networking-hyperv: | |
assignee: | nobody → Claudiu Belu (cbelu) |
Changed in os-win: | |
assignee: | nobody → Claudiu Belu (cbelu) |
Changed in oslotest: | |
assignee: | nobody → Claudiu Belu (cbelu) |
Changed in nova: | |
status: | New → In Progress |
Changed in nova: | |
assignee: | Claudiu Belu (cbelu) → Stephen Finucane (stephenfinucane) |
Changed in nova: | |
assignee: | Stephen Finucane (stephenfinucane) → Claudiu Belu (cbelu) |
Changed in os-win: | |
status: | New → In Progress |
Changed in nova: | |
assignee: | Matthew Edmonds (edmondsw) → Claudiu Belu (cbelu) |
Changed in nova: | |
assignee: | Claudiu Belu (cbelu) → Stephen Finucane (stephenfinucane) |
Changed in nova: | |
assignee: | Stephen Finucane (stephenfinucane) → Takashi NATSUME (natsume-takashi) |
Changed in nova: | |
assignee: | Takashi NATSUME (natsume-takashi) → Eric Fried (efried) |
Changed in nova: | |
assignee: | Eric Fried (efried) → Takashi NATSUME (natsume-takashi) |
Fix proposed to branch: master /review. openstack. org/524438
Review: https:/