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

--pyargs does not understand namespace packages #478

Open
pytestbot opened this Issue Mar 6, 2014 · 13 comments

Comments

Projects
None yet
7 participants
@pytestbot

pytestbot commented Mar 6, 2014

Originally reported by: Wolfgang Schnerring (BitBucket: wosc, GitHub: wosc)


The pyargs resolution does not understand namespace packages when the different contributing packages are installed as eggs (which is the layout used by zc.buildout and also pip install --egg, see http://peak.telecommunity.com/DevCenter/setuptools#namespace-packages). It looks like this:

site-packages/
    zope.asdf.egg/
        zope/
            asdf/
                ...
    zope.qwer.egg/
        zope/
            qwer/
                ...

This is a well supported layout, which means import zope.asdf and import zope.qwer work just fine (the specification is something to the effect of, if there is more than one directory that claims to be package "zope", there is no guarantee which one you will actually get when you say import zope, but all subpackages will be accessible regardless).

But py.test --pyargs zope.asdf will work, while py.test --pyargs zope.qwer will say "file or package not found" (it might also be the other way around, so zope.qwer works, but zope.asdf doesn't).

This is because _pytest.main.Session._tryconvertpyarg does not actually rely on the Python import mechanism to do the resolution (I'm not sure why, I'm guessing it's to support collecting tests outside of packages?). Instead it splits the argument on dots and loads the parts from the import system, assuming their filesystem location is the one that matters -- which is incorrect. In the example, trying to resolve zope.qwer, it will first resolve zope, which results in a random matching entry, e.g site-packages/zope.asdf/zope/__init__.py. It then assumes that the rest of the name must exist below this specific directory, thus never finding zope.qwer.


@pytestbot pytestbot added the type: bug label Jun 15, 2015

@scottpurdy

This comment has been minimized.

scottpurdy commented Sep 12, 2015

The relevant code is here.

Instead of iteratively importing from the top level package, could it try importing from the full path first and, if it fails, iteratively pop the rightmost element off until it succeeds in importing and then do the file lookup from there?

I'm happy to implement a solution if there is a reasonable option available.

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Sep 12, 2015

Since this is a setuptools mess, how about supporting module.path:object.path just like entry points of setuptools?

@scottpurdy

This comment has been minimized.

scottpurdy commented Sep 14, 2015

That seems fine to me. Does it work for packages though? I don't have a strong opinion on the discovery mechanism as long as it supports namespace packages.

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Sep 14, 2015

not sure off-hand, i'll probably need a while before i can touch it due to volunteering time constraints

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Apr 20, 2016

@scottpurdy is this sill an issue, with the gradual phasing out of eggs im thinking of letting this one be unless someone makes a pr

@scottpurdy

This comment has been minimized.

scottpurdy commented Apr 20, 2016

Not super high priority for me or I'd help out. Are eggs really being phased out?

@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Apr 21, 2016

yes, its no longer part of the equation in modern packaging tools
the common standard way of installation is pip+wheel

@eeaston

This comment has been minimized.

eeaston commented May 23, 2016

.. unless you want to support zip importing, which is really important for slow filesystems like NFS. This bug is related to #1445 - the test collector doesn't support zipfile importing either.
If I get some time soon I'll have a go at fixing this, the only workaround at the moment I can find is:

easy_install --always-unzip zope.asdf.egg
py.test $(python -c "import pkg_resources; print pkg_resources.get_distribution('zope.asdf').location")
@enkore

This comment has been minimized.

enkore commented Nov 2, 2016

This is kinda easy to trip over when using --pyargs, since one forgotten __init__.py lets py.test miraculously gloss over packages.

If a fix is complex, maybe just print a hint? Right now it says:

ERROR: file or package not found: somepkg

When we're on a Python supporting NS packages and --pyargs was used it could say something like...

ERROR: file or package not found: somepkg (missing __init__.py?)
@RonnyPfannschmidt

This comment has been minimized.

Member

RonnyPfannschmidt commented Nov 2, 2016

@enkore very good point

DuncanBetts pushed a commit to DuncanBetts/pytest that referenced this issue Nov 24, 2016

@DuncanBetts

This comment has been minimized.

Contributor

DuncanBetts commented Nov 24, 2016

I have made a Pull Request #2081 for commit ba3c1d2. Apologies if I should have waited for further discussion.

@nicoddemus

This comment has been minimized.

Member

nicoddemus commented Nov 24, 2016

Apologies if I should have waited for further discussion.

Not at all. It's no harm opening a PR, even if to entice more discussion. But I think adding just the message should be enough for this issue for now.

DuncanBetts added a commit to DuncanBetts/pytest that referenced this issue Nov 24, 2016

DuncanBetts added a commit to DuncanBetts/pytest that referenced this issue Nov 25, 2016

DuncanBetts added a commit to DuncanBetts/pytest that referenced this issue Nov 25, 2016

DuncanBetts added a commit to DuncanBetts/pytest that referenced this issue Nov 26, 2016

DuncanBetts added a commit to DuncanBetts/pytest that referenced this issue Nov 26, 2016

DuncanBetts added a commit to DuncanBetts/pytest that referenced this issue Nov 26, 2016

DuncanBetts added a commit to DuncanBetts/pytest that referenced this issue Nov 26, 2016

DuncanBetts added a commit to DuncanBetts/pytest that referenced this issue Nov 26, 2016

nicoddemus added a commit that referenced this issue Nov 26, 2016

Merge pull request #2085 from DuncanBetts/master
Add hint of Issue #478 to error text
@nicoddemus

This comment has been minimized.

Member

nicoddemus commented Nov 26, 2016

#2085 (thanks @DuncanBetts) adds the warning message at least.

This was referenced Mar 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment