Skip to content
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

Unittest discover fails with namespace packages and builtin modules #61659

Closed
PCManticore mannequin opened this issue Mar 18, 2013 · 20 comments
Closed

Unittest discover fails with namespace packages and builtin modules #61659

PCManticore mannequin opened this issue Mar 18, 2013 · 20 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@PCManticore
Copy link
Mannequin

PCManticore mannequin commented Mar 18, 2013

BPO 17457
Nosy @ericvsmith, @ezio-melotti, @voidspace, @PCManticore, @ericsnowcurrently
Files
  • unittest.patch
  • unittest-17457.patch
  • unittest-17457-2.patch
  • unittest-17457-3.patch
  • unittest_discovery_spec2.patch
  • unittest_discover.patch
  • namespace.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/voidspace'
    closed_at = <Date 2014-06-10.08:05:08.997>
    created_at = <Date 2013-03-18.12:15:30.127>
    labels = ['type-bug', 'library']
    title = 'Unittest discover fails with namespace packages and builtin modules'
    updated_at = <Date 2014-06-10.08:05:08.997>
    user = 'https://github.com/PCManticore'

    bugs.python.org fields:

    activity = <Date 2014-06-10.08:05:08.997>
    actor = 'berker.peksag'
    assignee = 'michael.foord'
    closed = True
    closed_date = <Date 2014-06-10.08:05:08.997>
    closer = 'berker.peksag'
    components = ['Library (Lib)']
    creation = <Date 2013-03-18.12:15:30.127>
    creator = 'Claudiu.Popa'
    dependencies = []
    files = ['29441', '29556', '32664', '32697', '32771', '32783', '32842']
    hgrepos = []
    issue_num = 17457
    keywords = ['patch']
    message_count = 20.0
    messages = ['184450', '185075', '185102', '192770', '192792', '202540', '203112', '203126', '203230', '203328', '203330', '203332', '203731', '203891', '203998', '204005', '204006', '204408', '212517', '220139']
    nosy_count = 6.0
    nosy_names = ['eric.smith', 'ezio.melotti', 'michael.foord', 'Claudiu.Popa', 'python-dev', 'eric.snow']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue17457'
    versions = ['Python 3.3', 'Python 3.4']

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Mar 18, 2013

    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.

    @PCManticore PCManticore mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Mar 18, 2013
    @voidspace
    Copy link
    Contributor

    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?

    @voidspace voidspace self-assigned this Mar 23, 2013
    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Mar 23, 2013

    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))

    1. 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.

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Jul 9, 2013

    Hello. Can I do something to move this issue forward?

    @voidspace
    Copy link
    Contributor

    I'd like to review this properly before committing it. I agree it solves a real problem and your new patch looks good.

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Nov 10, 2013

    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.

    @ezio-melotti
    Copy link
    Member

    I left a couple of comments on rietveld.

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Nov 17, 2013

    Attached new patch, which addresses Ezio's comments on Rietveld.

    @voidspace
    Copy link
    Contributor

    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.

    @ericsnowcurrently
    Copy link
    Member

    I left a review relative to the use of the _path attribute (which shouldn't be used).

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Nov 18, 2013

    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.

    @ericsnowcurrently
    Copy link
    Member

    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.

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Nov 22, 2013

    Hello! Attached patch which uses ModuleSpec, tested with http://hg.python.org/features/pep-451/ repo.

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Nov 22, 2013

    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.

    @voidspace
    Copy link
    Contributor

    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.

    @voidspace
    Copy link
    Contributor

    Need doc updates

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 23, 2013

    New changeset d2e5b74e2d18 by Michael Foord in branch 'default':
    bpo-17457: extend test discovery to support namespace packages
    http://hg.python.org/cpython/rev/d2e5b74e2d18

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Nov 25, 2013

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 1, 2014

    New changeset 57cb8a6e8f10 by R David Murray in branch 'default':
    whatsnew: unittest discover works on namespace packages (bpo-17457).
    http://hg.python.org/cpython/rev/57cb8a6e8f10

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Jun 10, 2014

    Can we close this? The feature already landed in Python 3.4.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants