Merge pull request #1891 from nicoddemus/find-spec-rewrite-hook

Fix issue pytest_plugins as string was marking wrong modules for rewrite

CHANGELOG.rst

@@ -10,7 +10,13 @@
   Thanks `@joguSD`_ for the PR.
 
 * Fix ``UnicodeEncodeError`` when string comparison with unicode has failed. (`#1864`_)
-  Thanks `@AiOO`_ for the PR
+  Thanks `@AiOO`_ for the PR.
+
+* ``pytest_plugins`` is now handled correctly if defined as a string (as opposed as
+  a sequence of strings) when modules are considered for assertion rewriting.
+  Due to this bug, much more modules were being rewritten than necessary
+  if a test suite uses ``pytest_plugins`` to load internal plugins (`#1888`_).
+  Thanks `@jaraco`_ for the report and `@nicoddemus`_ for the PR (`#1891`_).
 
 *
 
@@ -19,6 +25,8 @@
 
 .. _#1857: https://github.com/pytest-dev/pytest/issues/1857
 .. _#1864: https://github.com/pytest-dev/pytest/issues/1864
+.. _#1888: https://github.com/pytest-dev/pytest/issues/1888
+.. _#1891: https://github.com/pytest-dev/pytest/pull/1891
 
 
 3.0.1

_pytest/assertion/__init__.py

@@ -36,7 +36,13 @@ def register_assert_rewrite(*names):
     Thus you should make sure to call this before the module is
     actually imported, usually in your __init__.py if you are a plugin
     using a package.
+
+    :raise TypeError: if the given module names are not strings.
     """
+    for name in names:
+        if not isinstance(name, str):
+            msg = 'expected module names as *args, got {0} instead'
+            raise TypeError(msg.format(repr(names)))
     for hook in sys.meta_path:
         if isinstance(hook, rewrite.AssertionRewritingHook):
             importhook = hook

_pytest/config.py

@@ -379,6 +379,8 @@ def consider_env(self):
 
     def consider_module(self, mod):
         plugins = getattr(mod, 'pytest_plugins', [])
+        if isinstance(plugins, str):
+            plugins = [plugins]
         self.rewrite_hook.mark_rewrite(*plugins)
         self._import_plugin_specs(plugins)
 

testing/test_assertion.py

@@ -84,6 +84,29 @@ def test_foo(check_first):
             assert 0
         result.stdout.fnmatch_lines([expected])
 
+    @pytest.mark.parametrize('mode', ['str', 'list'])
+    def test_pytest_plugins_rewrite_module_names(self, testdir, mode):
+        """Test that pluginmanager correct marks pytest_plugins variables
+        for assertion rewriting if they are defined as plain strings or
+        list of strings (#1888).
+        """
+        plugins = '"ham"' if mode == 'str' else '["ham"]'
+        contents = {
+            'conftest.py': """
+                pytest_plugins = {plugins}
+            """.format(plugins=plugins),
+            'ham.py': """
+                import pytest
+            """,
+            'test_foo.py': """
+                def test_foo(pytestconfig):
+                    assert 'ham' in pytestconfig.pluginmanager.rewrite_hook._must_rewrite
+            """,
+        }
+        testdir.makepyfile(**contents)
+        result = testdir.runpytest_subprocess('--assert=rewrite')
+        assert result.ret == 0
+
     @pytest.mark.parametrize('mode', ['plain', 'rewrite'])
     def test_installed_plugin_rewrite(self, testdir, mode):
         # Make sure the hook is installed early enough so that plugins
@@ -196,6 +219,12 @@ def test_other():
                                      '>*assert l.pop() == 3*',
                                      'E*AssertionError'])
 
+    def test_register_assert_rewrite_checks_types(self):
+        with pytest.raises(TypeError):
+            pytest.register_assert_rewrite(['pytest_tests_internal_non_existing'])
+        pytest.register_assert_rewrite('pytest_tests_internal_non_existing',
+                                       'pytest_tests_internal_non_existing2')
+
 
 class TestBinReprIntegration: