gh-80958: Restore unittest discovery support for namespace packages as start directory - #123820
Conversation
Sorry, something went wrong.
|
@ericvsmith, you said in #68070 (comment):
Had your stance changed since then? do we revert it and make it work? or should we just only focus on improving the error message? |
Sorry, something went wrong.
|
I'm eager to hear Eric's view also. I'll just note that when the directory is provided explicitly as described in #80958, that strikes me as importantly distinct from an apporach that assumes every directory is/could be a namespace package. The revert (and subsequent fix) here only solves the explicit discovery issue, not the implicit discovery issue which is still a silent user error either way. |
Sorry, something went wrong.
…e packages Improve discovery
methane
left a comment
There was a problem hiding this comment.
start_dir can be namespace package. But it must not affect how treat subdirs.
Sorry, something went wrong.
|
Would you add a test case? $ find foo foo foo/noop foo/noop/test_noop.py foo/noop/no2 foo/noop/no2/__init__.py foo/noop/no2/test_no2.py foo/test_foo.py foo/bar foo/bar/__init__.py foo/bar/test_bar.py when unittest discover foo is executed, only test_foo and test_bar are discovered. |
Sorry, something went wrong.
| To avoid scanning directories unrelated to Python, | ||
| tests are not searched in subdirectories that do not contain ``__init__.py``. |
There was a problem hiding this comment.
Thanks, this is great 👍
Sorry, something went wrong.
Restore unittest discovery support for PEP 420 namespace packages as start directory (-s).
Rationale
There are two flavors of failures related to unittest discovery of namespace packages:
The silent failure ticket was marked as a duplicate of #68070, which was closed by a commit that removed support for namespace package discovery, but that solved neither issue. We still have a silent failure for user error (which I suppose is fine) and we still have a cryptic failure if a user attempts to provide it specifically (which is now user error according to the 3.11 revert).
We should do better than the cryptic error. But in looking into it, it seems a shame to give up so quickly when all the patch for #80958 needed to do was to cope with __name__ being None and __loader__ being set.
Notes
Re: the comment about this code being too hard to maintain: I think this regressed because of the heavy use of mocking in test_discovery.py. With real types this would have been caught by the existing test.
📚 Documentation preview 📚: https://cpython-previews--123820.org.readthedocs.build/