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 assertion rewriting module detection for egg dists #6313

Merged
merged 1 commit into from Dec 6, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/6301.bugfix.rst
@@ -0,0 +1 @@
Fix assertion rewriting for egg-based distributions and ``editable`` installs (``pip install --editable``).
53 changes: 52 additions & 1 deletion src/_pytest/config/__init__.py
Expand Up @@ -630,16 +630,67 @@ def __repr__(self):


def _iter_rewritable_modules(package_files):
"""
Given an iterable of file names in a source distribution, return the "names" that should
be marked for assertion rewrite (for example the package "pytest_mock/__init__.py" should
be added as "pytest_mock" in the assertion rewrite mechanism.
This function has to deal with dist-info based distributions and egg based distributions
(which are still very much in use for "editable" installs).
Here are the file names as seen in a dist-info based distribution:
pytest_mock/__init__.py
pytest_mock/_version.py
pytest_mock/plugin.py
pytest_mock.egg-info/PKG-INFO
Here are the file names as seen in an egg based distribution:
src/pytest_mock/__init__.py
src/pytest_mock/_version.py
src/pytest_mock/plugin.py
src/pytest_mock.egg-info/PKG-INFO
LICENSE
setup.py
We have to take in account those two distribution flavors in order to determine which
names should be considered for assertion rewriting.
More information:
https://github.com/pytest-dev/pytest-mock/issues/167
"""
package_files = list(package_files)
seen_some = False
for fn in package_files:
is_simple_module = "/" not in fn and fn.endswith(".py")
is_package = fn.count("/") == 1 and fn.endswith("__init__.py")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm the existing code seems to be a pre-PEP420 world, I wonder if we could update this to not be so dependent on __init__.py files

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, pytest itself doesn't seem to support this and AFAIR many tools were struggling to add support to this as well...

if is_simple_module:
module_name, _ = os.path.splitext(fn)
yield module_name
# we ignore "setup.py" at the root of the distribution
if module_name != "setup":
seen_some = True
yield module_name
elif is_package:
package_name = os.path.dirname(fn)
seen_some = True
yield package_name

if not seen_some:
# at this point we did not find any packages or modules suitable for assertion
# rewriting, so we try again by stripping the first path component (to account for
# "src" based source trees for example)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should not do this arbitrarily -- src is just a use of package_dir but it could be anywhere or any number of directories deep

perhaps a better approach would be to use sys.path to determine whether they are modules or not?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should not do this arbitrarily -- src is just a use of package_dir but it could be anywhere or any number of directories deep

That's just a comment, we are not hard-coding src anywhere... we do strip the first directory (if we don't find a suitable mode/package) and try again in the next iteration.

# this approach lets us have the common case continue to be fast, as egg-distributions
# are rarer
new_package_files = []
for fn in package_files:
parts = fn.split("/")
new_fn = "/".join(parts[1:])
if new_fn:
new_package_files.append(new_fn)
if new_package_files:
yield from _iter_rewritable_modules(new_package_files)


class Config:
"""
Expand Down
16 changes: 11 additions & 5 deletions testing/test_config.py
Expand Up @@ -422,15 +422,21 @@ def test_confcutdir_check_isdir(self, testdir):
@pytest.mark.parametrize(
"names, expected",
[
# dist-info based distributions root are files as will be put in PYTHONPATH
(["bar.py"], ["bar"]),
(["foo", "bar.py"], []),
(["foo", "bar.pyc"], []),
(["foo", "__init__.py"], ["foo"]),
(["foo", "bar", "__init__.py"], []),
(["foo/bar.py"], ["bar"]),
(["foo/bar.pyc"], []),
(["foo/__init__.py"], ["foo"]),
(["bar/__init__.py", "xz.py"], ["bar", "xz"]),
(["setup.py"], []),
# egg based distributions root contain the files from the dist root
(["src/bar/__init__.py"], ["bar"]),
(["src/bar/__init__.py", "setup.py"], ["bar"]),
(["source/python/bar/__init__.py", "setup.py"], ["bar"]),
],
)
def test_iter_rewritable_modules(self, names, expected):
assert list(_iter_rewritable_modules(["/".join(names)])) == expected
assert list(_iter_rewritable_modules(names)) == expected


class TestConfigFromdictargs:
Expand Down