Ensure that a module within a namespace package can be found by --pyargs

[taschini]

This PR addresses the problem highlighted in #1567 and in part #478.

The function test_cmdline_python_namespace_package in acceptance_test.py tests and demonstrates the fix. First it creates the following tree where ns_pkg is a namespace package:

        .
        ├── hello
        │   └── ns_pkg
        │       ├── __init__.py
        │       └── hello
        │           ├── __init__.py
        │           └── test_hello.py
        └── world
            └── ns_pkg
                ├── __init__.py
                └── world
                    ├── __init__.py
                    └── test_world.py

Then it verifies that with hello and world added the python path the following commands works as expected::

$ py.test -v --pyargs ns_pkg.hello
...
collected 2 items 

hello/ns_pkg/hello/test_hello.py::test_hello PASSED
hello/ns_pkg/hello/test_hello.py::test_other PASSED
…

$ py.test -v --pyargs ns_pkg.world
…
collected 2 items 

world/ns_pkg/world/test_world.py::test_world PASSED
world/ns_pkg/world/test_world.py::test_other PASSED
…

For the time being I came to the conclusion that if you specify the namespace package itself, the safest solution is to explicitly flag it as an error:

$ py.test -q --pyargs ns_pkg
no tests ran in 0.00 seconds
ERROR: Cannot uniquely resolve package directory: ns_pkg

[taschini]

CI caught a few small mistakes. I'll fix them as soon as possible.

[coveralls]

Coverage increased (+0.007%) to 92.227% when pulling 11b04e0 on taschini:namespaces into 436e13a on pytest-dev:master.

[taschini]

This PR now satisfies the test-suite, but the solution I went for is not overly robust. The only reliable way to determine if a package is a namespace package is to import it. I did shy away from importing anything at the argument parsing stage, but the result is a bit fragile. To have a reliable solution, one must either import the package during argument parsing or defer --pyargs argument conversion to a later stage.

Any feedback?

[RonnyPfannschmidt]

IMHO import for pyarg usage is acceptable

as other consideraion, we could also change how/when pyargs are considered (i.e. move them to the collect stage)

in any case i think putting this in as a "experimental" "feature" might make sense

where the experimental flag is mainly used to allow for removal in case we go for the different consideration later on

@The-Compiler @nicoddemus please lean in your opinions as well

[The-Compiler]

I can't say much, I've never used --pyargs and I'm not really familiar with namespace packages either 😉

[nicoddemus]

Could you please put a comment explaining why do you check for the file size at this point?

[nicoddemus]

Hi @taschini, first of all thanks for the work! 😁

I made a few comments on the PR, I'm mostly curious why the getsize() == 0 check, other than that the rest are small nitpicks.

I've never used --pyargs myself, but from what I've seen it looks good so far.

[coveralls]

Coverage increased (+0.007%) to 92.227% when pulling 811f837 on taschini:namespaces into 436e13a on pytest-dev:master.

[taschini]

Hi @nicoddemus, I incorporated your feedback in the latest commit.

Now, as for getsize() == 0, as I was saying before, the only reliable way to determine whether a package is a namespace package or a regular package is to import it. Trying to emulate the import machinery will inevitably leave some corner cases exposed.

Even leaving aside this PR, emulation using imp.find_module is guaranteed to leave some cases out in Python versions implementing PEP 420:

$ mkdir -p foo/bar
$ touch foo/bar/__init__.py

$ python2.7 -c 'import foo.bar; print(foo.bar)'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: No module named foo.bar

$ python3.5 -c 'import foo.bar; print(foo.bar)'
<module 'foo.bar' from '.../foo/bar/__init__.py'>

$ py.test-3.5 --pyarg foo.bar
================================================================ test session starts ================================================================
platform darwin -- Python 3.5.1, pytest-2.9.1, py-1.4.31, pluggy-0.3.1
rootdir: .../pytest_bug4, inifile: 
plugins: cov-2.2.1, remove-stale-bytecode-2.1

=========================================================== no tests ran in 0.00 seconds ============================================================
ERROR: file or package not found: foo.bar

So the ultimate resolution to #1567 will need to import the packages, either straight away at the argument parsing stage or at the collection stage, by delaying the handling of --pyargs.

In this PR, which I started before all the intricacies of namespace packages were clear to me, I was trying to stay away from importing the package, and I tried to implement a conservative emulation that would throw an exception in case of doubt. As it turns out, doing so naïvely would let py.test give up on the directory resolution of packages too often, so I added a little heuristic: if the __init__.py file of a package exists and is empty, then it is a regular package, not a namespace package.

The question is now how to go forward: whether you guys accept this PR as a preliminary solution or reject it, and which the two alternatives:

1. Importing during argument parsing,
2. Handling of --pyargs switch at the collection stage,

is preferrable.

[nicoddemus]

Hi @taschini, thanks for implementing the changes and further clarification of the issue, it is an interesting reading as I never used namespace packages myself.

While I agree with the reasoning that it is acceptable to import the package during argument parsing, I think it would fit better overall to handle --pyargs during collection, because at that point is easier to report proper collection errors than at the argument parsing stage. Not sure how simple would be to implement that though.

Apart from that, do you guys feel that perhaps the current solution is good enough, or would leave too much corner cases uncovered (I ask because it is not clear to me when you said "will inevitably leave some corner cases exposed." you meant before using imp.find_module or after)?

[taschini]

I created a new pull request (#1597) that uses pkgutil to do mapping of a module name to file path.