yahoo-eng-team team mailing list archive

[Claudiu Belu <cbelu@xxxxxxxxxxxxxxxxxxxxxx>]

Public bug reported:

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

** Affects: networking-hyperv
     Importance: Undecided
     Assignee: Claudiu Belu (cbelu)
         Status: New

** Affects: nova
     Importance: Undecided
     Assignee: Claudiu Belu (cbelu)
         Status: New

** Affects: os-win
     Importance: Undecided
     Assignee: Claudiu Belu (cbelu)
         Status: New

** Affects: oslotest
     Importance: Undecided
     Assignee: Claudiu Belu (cbelu)
         Status: In Progress


** Tags: testing

** Also affects: oslotest
   Importance: Undecided
       Status: New

** Also affects: os-win
   Importance: Undecided
       Status: New

** Also affects: networking-hyperv
   Importance: Undecided
       Status: New

** Changed in: nova
     Assignee: (unassigned) => Claudiu Belu (cbelu)

** Changed in: networking-hyperv
     Assignee: (unassigned) => Claudiu Belu (cbelu)

** Changed in: os-win
     Assignee: (unassigned) => Claudiu Belu (cbelu)

** Changed in: oslotest
     Assignee: (unassigned) => Claudiu Belu (cbelu)

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Compute (nova).
https://bugs.launchpad.net/bugs/1735588

Title:
  Mock autospec not used, or not used properly

Status in networking-hyperv:
  New
Status in OpenStack Compute (nova):
  New
Status in os-win:
  New
Status in oslotest:
  In Progress

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(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

To manage notifications about this bug go to:
https://bugs.launchpad.net/networking-hyperv/+bug/1735588/+subscriptions