Add H216 to flag use of third party mock

Many projects use mocking in their unit tests, and most do not realize
that there is a difference between "import mock" and "import
unittest.mock", assuming that both use a standard part of the Python
library.

We've seen many cases where mock is not listed in the project's
requirements, but the code imports the third party mock instead of
unittest.mock. We've also seen a few break due to this, once their
dependencies have stopped pulling in that package for them.

There have also been several projects that have taken the effort to
switch all of there "import mock" statements over to "import
unittest.mock", as well as removing mock from their requirements, only
to then accidentally merge a patch that does "import mock" again because
it is hard to notice in code reviews.

This check is on by default. If a project is using the mock lib, then
they are able to explicitly do so by disabling this check. Otherwise,
projects don't need to take any action to get this protection, since
this is now the recommended default.

Change-Id: I8d255a00792a19279074703a8209a3699b480fd0
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>

HACKING.rst

@@ -361,6 +361,24 @@ exception possible should be used.
   assertEqual(A in B, False) or assertEqual(False, A in B) to the more
   specific assertIn/NotIn(A, B)
 
+- [H216] Make sure unittest.mock is used instead of the third party mock
+  library. On by default.
+
+  Starting with Python 3.3 and later, the mock module was added under unittest.
+  Previously, this functionality was only available by using the third party
+  ``mock`` library.
+
+  Most users are not aware of this subtle distinction. This results in issues
+  where the project does not declare the ``mock`` library in its requirements
+  file, but the code does an ``import mock`` assuming that the module is
+  present. This may work initially if one of the project's dependencies ends up
+  pulling that dependency in indirectly, but then can cause things to suddenly
+  break if that transitive dependency goes away.
+
+  Since this third party library usage is done without being aware of it, this
+  check is enabled by default to make sure those projects that actually do
+  intend to use the ``mock`` library are doing so explicitly.
+
 OpenStack Trademark
 -------------------
 

hacking/checks/mock_checks.py

@@ -11,6 +11,7 @@
 #  under the License.
 
 import ast
+import re
 
 from hacking import core
 
@@ -139,3 +140,37 @@ class FunctionNameFinder(ast.NodeVisitor):
         if isinstance(node, ast.Call):
             return super(FunctionNameFinder, self).visit(node.func)
         return super(FunctionNameFinder, self).visit(node)
+
+
+third_party_mock = re.compile('^import.mock')
+from_third_party_mock = re.compile('^from.mock.import')
+
+
+@core.flake8ext
+def hacking_no_third_party_mock(logical_line, noqa):
+    """Check for use of mock instead of unittest.mock.
+
+    Projects have had issues with using mock without including it in their
+    requirements, thinking it is the standard library version. This makes it so
+    these projects need to explicitly turn off this check to make it clear they
+    intended to use it.
+
+    Okay: from unittest import mock
+    Okay: from unittest.mock import patch
+    Okay: import unittest.mock
+    H216: import mock
+    H216: from mock import patch
+    Okay: try: import mock
+    Okay: import mock  # noqa
+    """
+    msg = ('H216: The unittest.mock module should be used rather than the '
+           'third party mock package unless actually needed. If so, disable '
+           'the H216 check in hacking config and ensure mock is declared in '
+           "the project's requirements.")
+
+    if noqa:
+        return
+
+    if (re.match(third_party_mock, logical_line) or
+            re.match(from_third_party_mock, logical_line)):
+        yield (0, msg)

hacking/tests/checks/test_except_checks.py

@@ -12,8 +12,8 @@
 # limitations under the License.
 
 import textwrap
+from unittest import mock
 
-import mock
 import pycodestyle
 
 from hacking.checks import except_checks

hacking/tests/checks/test_mock.py

@@ -0,0 +1,37 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import ddt
+
+from hacking.checks import mock_checks
+from hacking import tests
+
+
+@ddt.ddt
+class MockingTestCase(tests.TestCase):
+    """This tests hacking checks related to the use of mocking."""
+
+    @ddt.unpack
+    @ddt.data(
+        (1, 'import mock', None),
+        (0, 'from unittest import mock', None),
+        (1, 'from mock import patch', None),
+        (0, 'from unittest.mock import patch', None),
+        (0, 'from mock', '# noqa'))
+    def test_H216_hacking_no_third_party_mock(self, err_count, line, noqa):
+        if err_count > 0:
+            self.assertCheckFails(mock_checks.hacking_no_third_party_mock,
+                                  line, noqa)
+        else:
+            self.assertCheckPasses(mock_checks.hacking_no_third_party_mock,
+                                   line, noqa)

lower-constraints.txt

@@ -12,7 +12,6 @@ Jinja2==2.10
 linecache2==1.0.0
 MarkupSafe==1.0
 mccabe==0.6.0
-mock==3.0.0
 pycodestyle==2.4.0
 pyflakes==2.1.1
 Pygments==2.2.0

setup.cfg

@@ -45,6 +45,7 @@ flake8.extension =
     H213 = hacking.checks.except_checks:hacking_assert_raises_regexp
     H214 = hacking.checks.except_checks:hacking_assert_true_or_false_with_in
     H215 = hacking.checks.except_checks:hacking_assert_equal_in
+    H216 = hacking.checks.mock_checks:hacking_no_third_party_mock
     H231 = hacking.checks.python23:hacking_python3x_except_compatible
     H232 = hacking.checks.python23:hacking_python3x_octal_literals
     H233 = hacking.checks.python23:hacking_python3x_print_function

test-requirements.txt

@@ -3,7 +3,6 @@
 # process, which may cause wedges in the gate later.
 coverage!=4.4,>=4.0 # Apache-2.0
 fixtures>=3.0.0 # Apache-2.0/BSD
-mock>=3.0.0 # BSD
 python-subunit>=1.0.0 # Apache-2.0/BSD
 stestr>=2.0.0 # Apache-2.0
 testscenarios>=0.4 # Apache-2.0/BSD