Configuring discovery for paths with namespace packages

[0cjs]

The structure of Python namespace packages cannot be determined just by looking at the filesystem. Consider the following directory tree:

src/
    core.py
    aaa/
        test_core.py
    bbb/
        core.py
        test_core.py

If src/ (but nothing underneath it) is in sys.path, we have the following modules:

core
aaa
aaa.test_core
bbb
bbb.core
bbb.test_core

where aaa and bbb are namespace modules. We might also have other parts of the namespace modules outside of this tree, e.g. by adding othersrc/ to sys.path so that import bbb.other will work if othersrc/bbb/other.py exists.)

It's easy to configure pytest to have src in the path, but when discovering and loading src/aaa/test_core.py, as described here, it adds src/aaa to sys.path and imports src/aaa/test_core.py using a different module structure than the programmer may have intended: import test_core. Further, the same thing happens again with src/bbb/test_core.py and now we get a conflict because there's already a test_core module.

It would be nice to be able to configure pytest so that we could tell it that src/ may contain namespace packages and, if it does, they are all intended to be rooted at src/. That is, when discovering src/{aaa,bbb}/test_core.py do not add anything to sys.path but instead generate the fully qualified module name based on the file's position in the hierarchy relative to src/.

Another, more aggressive, option might be to see if the file is under any of the paths already in sys.path and determine the fully qualified module name by the hierarchy relative to the first path in sys.path under which the file falls.

The Stack Overflow question How do I Pytest a project using PEP 420 namespace packages? also discusses the issue.

The way I put these here makes this sound pretty simple (at least as far as description, if not implementation), but there may be other subtleties I'm missing here, about which I welcome discussion.

[asottile]

I'd like to see pytest work better in this situation as well. I think once we can leave python2 behind this becomes even easier (and more common at the same time) as __init__.py files become entirely optional.

I'm not quite sure the best way to get to that state without breaking everything though 🤔

In my opinion ideally the "namespace package" situation would be the default, and the class of errors around test filename collision would disappear. Though I haven't put any thought or effort into seeing how this would work in code, @nicoddemus would know better

[0cjs]

I've done a test implementation based on hoefling's answer to the SO question above except instead of having an independently-specified list of namespace package dirs I simply use sys.path, returning the top-level package or dir under the first entry from sys.path that is above the package in question. If no entry in sys,path is above the package we're trying to load (say, becuase someone's asked pytest to discover tests in a directory outside of any in sys.path, I simply pass the path on to the old code and let it do its thing.

I've only given it brief testing, but it seems to be working pretty well. I will report back again after further testing.

I don't know why I didn't think of this before, because it actually seems to be the most obvious and sensible solution (or at least partial solution) to the issue since, when the module is under sys.path we're generating a name that would load that exact module if pytest weren't running. In somewhat pathological situations it might not be the name that the developer would use, but it will definitely not be a name that could ever be used by another module. And it also, when the module can be found in the path, avoids the confusing and shadowing created by adding more locations to the path. (I use --import-mode=append, but even I've been bitten by a module unexpectedly loading in tests when it wouldn't in production.)

My current thinking on the matter is that py.path.LocalPath.pypkgpath() should take an optional second argument (defaulting to an empty list when not provided) that would be the list of directories to consider potential roots of namespace modules. You could pass in nothing to preserve the current behaviour, sys.path to get the (I think sensible) behaviour described here, or any other paths you care to configure. Getting that argument down to it probably means also adding it to pyimport() in the same module, but after that we're out of py-land and in pytest code so the rest is a matter of adding new configuration.

While using sys.path in this way actually seems like sensible default behaviour to me, it might be something we'd want either to phase in gradually (add the option first, and make it the default a few versions later) or just leave with a seemingly-poor default in the name of backward compatibility.

[girtsf]

I just ran into the same thing. Something like a command line option that could be added to pytest.ini to force package imports from rootdir sounds good to me.

[0cjs]

I don't think that using rootdir for this is a good idea: rootdir is about configuration, not import path, and can even change from run to run depending on your current working directory and what paths you give to pytest for discovery.

BTW, for those wondering where I'm at with all of this, my hack above seems to have worked well over the initial week since I implemented it, but the last ten days have been a long holiday in Japan (Golden Week) and so all work on the project using this has stopped.

Assuming my change goes for another week or so without any issues, I will look at trying to find the time to code up a PR for this. (It's unfortunately not trivial in part because it requires changes to both py and pytest.) If anybody's got any special thoughts on how to go about doing this beyond what any decent developer would do, let me know.

[girtsf]

I would love to help get a PR out if somebody pointed me in the right direction on what the recommended approach is.

[RonnyPfannschmidt]

we have a concept of "collection roots" floating around, however its not yet formalized

for me personally thats something i'd like to tackle sometime after sorting out the collection tree structure

many of the details are currently unclear to me

[Traktormaster]

Hi!
I've recently found out about pytest not respecting PEP420 namespaces.

To circumvent the issue I've patched the package discovery to treat paths that are already included in sys.path as namespaces. (essentially forcing import with fully qualified module name from a sys.path entry)
You can read my initial thoughts on it in detail here.

In my opinion it would be favorable to have valid PEP420 namespaces working as expected by default. With my patch, this would (I think) always be the case unless the sys.path is not set up correctly. By correctly, I mean that all places where one would import such packages are added to PYTHONPATH or sys.path directly.
One improvement on my patch would be to check if the directories in question have no files in them at all indeed. (make the PEP420 namespace check a little more correct)

[0cjs]

Using existing paths in sys.path seems a good solution for Python ≤3.4, where modules can be loaded only "through" a path. However, for 3.5 and above I would prefer to see discovery use the newer loading mechanisms that allow discovered test files to be loaded as "free-floating" modules (i.e., not entered at all in sys.modules) without them needing to go through a path to generate a module name for use in sys.modules, as I did in the code in #6966. This should avoid all conflicts and preserve the path that the tests were started with, rather than runnning tests in a system with modified path. (If tests were relying on the path being changed, the test system should be changed to make explicit that those paths are part of the test environment.)

[blueyed]

@0cjs
FWIW: it is also possible to restore sys.path after importing - but I agree that it would be better to skip/replace pylib's method here completely maybe.

[0cjs]

FWIW: it is also possible to restore sys.path after importing....

@blueyed That hadn't occurred to me! I wouldn't want to do that for Python ≥3.5, where we have better methods of dealing with that, but it seems to me a perfectly reasonable idea to consider when running earlier versions of Python, in the hope of maintaining more compatibility between pytest on ≤3.4 and ≥3.5.

I suppose this would also involve removing the module from sys.modules after loading it as well, to also emulate the behaviour of "nameless" modules, and avoid accidental "'loaded' from cache" for subsequent (but different) files that map to the same module name.

This would require a bit of thought, but possibly might work quite well.

[blueyed]

@0cjs
As for sys.path (from one of my stashes):

diff --git a/src/_pytest/python.py b/src/_pytest/python.py
index 372631f10..265bf0689 100644
--- a/src/_pytest/python.py
+++ b/src/_pytest/python.py
@@ -515,6 +515,7 @@ def _importtestmodule(self) -> ModuleType:
         # we assume we are only called once per module
         importmode = self.config.getoption("--import-mode")
         fspath = self.fspath
+        old_sys_path = sys.path[:] if importmode != "importlib" else None
         try:
             mod = fspath.pyimport(ensuresyspath=importmode)
         except SyntaxError:
@@ -554,6 +555,9 @@ def _importtestmodule(self) -> ModuleType:
                 "or @pytest.mark.skipif decorators instead, and to skip a "
                 "module use `pytestmark = pytest.mark.{skip,skipif}."
             )
+        finally:
+            if old_sys_path:
+                sys.path = old_sys_path
         self.config.pluginmanager.consider_module(mod)
         return mod

Feel free to pick it up into a PR.
Tested in blueyed#338 already (with some required test adjustments).

[blueyed]

As for pytest you do not have to consider anything before Python 3.5 btw.