classification
Title: Unittest discover fails with namespace packages and builtin modules
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.4, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: michael.foord Nosy List: Claudiu.Popa, eric.smith, eric.snow, ezio.melotti, michael.foord, python-dev
Priority: normal Keywords: patch

Created on 2013-03-18 12:15 by Claudiu.Popa, last changed 2014-06-10 08:05 by berker.peksag. This issue is now closed.

Files
File name Uploaded Description Edit
unittest.patch Claudiu.Popa, 2013-03-18 12:15 review
unittest-17457.patch Claudiu.Popa, 2013-03-23 22:16 review
unittest-17457-2.patch Claudiu.Popa, 2013-11-17 08:11 review
unittest-17457-3.patch Claudiu.Popa, 2013-11-18 22:48 review
unittest_discovery_spec2.patch Claudiu.Popa, 2013-11-22 10:17 review
unittest_discover.patch Claudiu.Popa, 2013-11-22 21:38 review
namespace.patch Claudiu.Popa, 2013-11-25 21:10 review
Messages (20)
msg184450 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2013-03-18 12:15
There is a problem with unittest discovering and namespace packages. Given the following folder structure, where a namespace package X lies, the following command fails with the following error:

-testbug
   - flufl (namespace package with some tests in it, importable with __import__)
      - test_a.py
      - test_b.py


C:\>py -3 -m unittest discover flufl
Traceback (most recent call last):
  File "C:\Python33\lib\runpy.py", line 160, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "C:\Python33\lib\runpy.py", line 73, in _run_code
    exec(code, run_globals)
  File "C:\Python33\lib\unittest\__main__.py", line 12, in <module>
    main(module=None)
  File "C:\Python33\lib\unittest\main.py", line 124, in __init__
    self.parseArgs(argv)
  File "C:\Python33\lib\unittest\main.py", line 144, in parseArgs
    self._do_discovery(argv[2:])
  File "C:\Python33\lib\unittest\main.py", line 242, in _do_discovery
    self.test = loader.discover(start_dir, pattern, top_level_dir)
  File "C:\Python33\lib\unittest\loader.py", line 205, in discover
    start_dir = os.path.abspath(os.path.dirname((the_module.__file__)))
AttributeError: 'module' object has no attribute '__file__'

This happens because TestLoader.discover assumes that the given dotted package name has the attribute __file__, which seems to not be true in the case of namespace packages. The same error occurs when giving to `discover` a builtin module.
The attached patch tries naively to solve this issue, but it assume in TestLoader._find_tests that it should iterate over all subfolders (the commented line from the patch), unlike the previous way of checking the presence of __init__.py file.
Thanks in advance for your response.
msg185075 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-03-23 19:05
Thanks for the report and the patch. Good catch!

It will need looking over as it's not immediately obvious to me it's correct. 

In the code that checks the loader path, does it iterate over every member of the namespace path - even directories that aren't in the path being searched?
msg185102 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2013-03-23 22:16
Yes, it iterates over every member of the namespace path. The new attached patch fixes this behaviour, by checking that each loader path starts with top_level_dir when set_implicit_top is False (if set_implicit_top is True, top_level_dir is irrelevant, start_dir will be the package name).

 Also, with the original patch there were other issues:
  1. if no tests were found for the namespace package, the same failure as before would have occurred at the lines:

  +        if not tests:
  +            tests = list(self._find_tests(start_dir, pattern))

  2. it iterated every subfolder, by dropping the check for __init__.py. To fix this, I added a new keyword argument to _find_tests, `namespace` which defaults to False. If it is True, then we are checking a namespace package and subfolders will be checked even if they don't have a __init__.py file.
msg192770 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2013-07-09 18:15
Hello. Can I do something to move this issue forward?
msg192792 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-07-10 07:49
I'd like to review this properly before committing it. I agree it solves a real problem and your new patch looks good.
msg202540 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2013-11-10 17:33
Michael, is any chance for this to go into Python 3.4? I would love to make any changes necessary in order for this to happen.
msg203112 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-11-17 00:43
I left a couple of comments on rietveld.
msg203126 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2013-11-17 08:11
Attached new patch, which addresses Ezio's comments on Rietveld.
msg203230 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-11-17 23:44
I do want to get this into 3.4. The logic is non-trivial though, so I need to understand it before I can add it to discovery. It certainly *looks* good.
msg203328 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-11-18 22:17
I left a review relative to the use of the _path attribute (which shouldn't be used).
msg203330 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2013-11-18 22:48
Eric, thank you for your comment! I modified the patch to check for __path__ instead, is this a safe enough check for this use case? Since ModuleSpec isn't landed yet, I didn't use your __spec__ example.
msg203332 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-11-18 23:51
Sorry for any confusion, Claudiu.  the_module.__path__ only indicates that the module is a package.  So until you can take advantage of PEP 451, you're stuck with the _path check (or several other unappealing hack) on line 224 of unittest-17457-3.patch.

However, the use of __path__ on line 226 is correct now. :)  Thanks for changing that.
msg203731 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2013-11-22 10:17
Hello! Attached patch which uses ModuleSpec, tested with http://hg.python.org/features/pep-451/ repo.
msg203891 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2013-11-22 21:38
Added patch which addresses Eric's comments. It contains a situation which I'm not so happy, mostly due to not knowing in depth (or at least at a comfortable level) the import mechanics: in the AttributeError except handler, if the spec can't be determined and the module is not built-in, a TypeError is raised to notice the user that discovery can't be done for that particular package. Probably this situation (no __file__, no __spec__ and no built-in) is common, but I can't think right now at another way of solving it.
msg203998 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-11-23 13:09
I'm going to commit this so we get it in before the feature freeze. It sounds like the remaining issue is minor and we can resolve it in the betas.

Note that if we attempt discovery from a namespace package and fail we should create a ModuleImportError test case to report the problem and *not* bomb out of test collection.
msg204005 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-11-23 13:25
Need doc updates
msg204006 - (view) Author: Roundup Robot (python-dev) Date: 2013-11-23 13:30
New changeset d2e5b74e2d18 by Michael Foord in branch 'default':
Issue 17457: extend test discovery to support namespace packages
http://hg.python.org/cpython/rev/d2e5b74e2d18
msg204408 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2013-11-25 21:10
Sorry for the delay. Here's a minimal doc patch. I don't excel at writing documentation, it's not quite my strong point, so feel free to modify anything you seem fit.
msg212517 - (view) Author: Roundup Robot (python-dev) Date: 2014-03-01 17:11
New changeset 57cb8a6e8f10 by R David Murray in branch 'default':
whatsnew: unittest discover works on namespace packages (#17457).
http://hg.python.org/cpython/rev/57cb8a6e8f10
msg220139 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-06-10 06:45
Can we close this? The feature already landed in Python 3.4.
History
Date User Action Args
2014-06-10 08:05:08berker.peksagsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2014-06-10 06:45:32Claudiu.Popasetmessages: + msg220139
2014-03-01 17:11:53python-devsetmessages: + msg212517
2013-11-25 21:10:41Claudiu.Popasetfiles: + namespace.patch

messages: + msg204408
2013-11-23 13:30:19python-devsetnosy: + python-dev
messages: + msg204006
2013-11-23 13:25:47michael.foordsetmessages: + msg204005
2013-11-23 13:09:41michael.foordsetmessages: + msg203998
2013-11-22 21:38:32Claudiu.Popasetfiles: + unittest_discover.patch

messages: + msg203891
2013-11-22 10:17:03Claudiu.Popasetfiles: + unittest_discovery_spec2.patch

messages: + msg203731
2013-11-18 23:51:07eric.snowsetmessages: + msg203332
2013-11-18 22:48:19Claudiu.Popasetfiles: + unittest-17457-3.patch

messages: + msg203330
2013-11-18 22:17:58eric.snowsetnosy: + eric.snow
messages: + msg203328
2013-11-17 23:44:28michael.foordsetmessages: + msg203230
2013-11-17 08:11:52Claudiu.Popasetfiles: + unittest-17457-2.patch

messages: + msg203126
2013-11-17 00:43:50ezio.melottisetmessages: + msg203112
2013-11-10 17:33:55Claudiu.Popasetmessages: + msg202540
2013-07-10 07:49:10michael.foordsetmessages: + msg192792
2013-07-09 18:15:20Claudiu.Popasetmessages: + msg192770
2013-03-23 22:16:35Claudiu.Popasetfiles: + unittest-17457.patch

messages: + msg185102
2013-03-23 19:05:35michael.foordsetassignee: michael.foord
messages: + msg185075
2013-03-23 14:15:48ezio.melottisetnosy: + ezio.melotti, michael.foord

stage: patch review
2013-03-18 15:03:57eric.smithsetnosy: + eric.smith
2013-03-18 12:15:30Claudiu.Popacreate