Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue pytest_plugins as string was marking wrong modules for rewrite #1891

Merged
merged 2 commits into from
Aug 31, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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`_).

*

Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions _pytest/assertion/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions _pytest/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
29 changes: 29 additions & 0 deletions testing/test_assertion.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:

Expand Down