TestLoader.loadTestsFromName swallows import errors

[rbtcollins]

BPO	7559
Nosy	@brettcannon, @rbtcollins, @mcepl, @ezio-melotti, @bitdancer, @voidspace, @cjerdonek, @Julian, @ericsnowcurrently, @berkerpeksag, @zware, @timgraham
Files	issuee7559_trunk_patch.diff: Patch against trunk (work in progress) issuee7559_trunk_patch.diff: Patch against trunk, adds new unittest. _patch-7559-2.diff: Patch against trunk (work in progress) _patch-7559-3.diff: Patch against trunk (needs review) _patch-7559-4-unittests.diff: Candidate unit tests (4 tests) _patch-7559-5.diff: Patch against trunk (minor clean-up needed) alternate_import_fix.patch _patch-7559-6.diff: Requesting review unittest_loader_import_error.patch issue7559.patch: Obsolete issue7559.patch issue7559.patch issue7559.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-10-29.19:29:09.736>
created_at = <Date 2009-12-21.21:07:15.519>
labels = ['easy', 'type-bug', 'library']
title = 'TestLoader.loadTestsFromName swallows import errors'
updated_at = <Date 2016-01-11.16:39:58.962>
user = 'https://github.com/rbtcollins'

bugs.python.org fields:

activity = <Date 2016-01-11.16:39:58.962>
actor = 'mcepl'
assignee = 'michael.foord'
closed = True
closed_date = <Date 2014-10-29.19:29:09.736>
closer = 'python-dev'
components = ['Library (Lib)']
creation = <Date 2009-12-21.21:07:15.519>
creator = 'rbcollins'
dependencies = []
files = ['15686', '15728', '16759', '16796', '16819', '16822', '16829', '16862', '25790', '36578', '36974', '37002', '37045']
hgrepos = []
issue_num = 7559
keywords = ['patch', 'easy']
message_count = 60.0
messages = ['96770', '96774', '96777', '96787', '96788', '96789', '96790', '96891', '96956', '97180', '97457', '97462', '101867', '102236', '102356', '102519', '102550', '102554', '102603', '102621', '102638', '102643', '102708', '102719', '102801', '140187', '140297', '140328', '158911', '158920', '158921', '162101', '162190', '162212', '162239', '162241', '162264', '162265', '162266', '162272', '185310', '185328', '185329', '221984', '221985', '226614', '226616', '226633', '226658', '226663', '226666', '226667', '229705', '229911', '230104', '230234', '230557', '230565', '230734', '244687']
nosy_count = 18.0
nosy_names = ['brett.cannon', 'rbcollins', 'vila', 'mcepl', 'iElectric', 'ezio.melotti', 'r.david.murray', 'michael.foord', 'slmnhq', 'chris.jerdonek', 'BreamoreBoy', 'Julian', 'python-dev', 'eric.snow', 'alexgarel', 'berker.peksag', 'zach.ware', 'Tim.Graham']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue7559'
versions = ['Python 3.5']

[rbtcollins]

Say I have a test module test_foo, which fails to import with
ImportError. A reason for this might be a misspelt import in that module.

TestLoader().loadTestsFromName swallows the import error and instead
crashes with:
File "/usr/lib/python2.6/unittest.py", line 584, in loadTestsFromName
parent, obj = obj, getattr(obj, part)
AttributeError: 'module' object has no attribute 'test_foo'

A better thing to do would be to keep the import error and if the next
probe is an Attribute error, reraise the import error.
An alternative would be to return a test which would then reraise the
import error permitting the test suite to be constructed and execute but
still reporting the error.

[bitdancer]

Could you provide a more complete recipe for reproducing the problem,
please? I created a test_foo.py containing 'import blert', and running
python -m unittest test_foo does not mask the import error for blert in
loadTestsFromName:

...
File "/usr/lib/python2.6/unittest.py", line 576, in loadTestsFromName
module = __import__('.'.join(parts_copy))
File "test_foo.py", line 1, in <module>
import blert
ImportError: No module named blert

[rbtcollins]

mkdir thing
touch thing/__init__.py
echo "import blert" > thing/test_foo.py
python -m unittest thing.test_fooTraceback (most recent call last):
  File "/usr/lib/python2.6/runpy.py", line 122, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python2.6/runpy.py", line 34, in _run_code
    exec code in run_globals
  File "/usr/lib/python2.6/unittest.py", line 875, in <module>
    main(module=None)
  File "/usr/lib/python2.6/unittest.py", line 816, in __init__
    self.parseArgs(argv)
  File "/usr/lib/python2.6/unittest.py", line 843, in parseArgs
    self.createTests()
  File "/usr/lib/python2.6/unittest.py", line 849, in createTests
    self.module)
  File "/usr/lib/python2.6/unittest.py", line 613, in loadTestsFromNames
    suites = [self.loadTestsFromName(name, module) for name in names]
  File "/usr/lib/python2.6/unittest.py", line 584, in loadTestsFromName
    parent, obj = obj, getattr(obj, part)
AttributeError: 'module' object has no attribute 'test_foo'

[bitdancer]

Thank you. I can reproduce this on trunk as well. I'm leaving stage
set to test needed because we need to turn this into a unit test.

[bitdancer]

Note: this problem is similar in some ways to bpo-5230, and a similar
solution might be appropriate (or might not :).

[voidspace]

I'll try and look at both these issues in the next few days unless one
of you beats me to it. :-)

[rbtcollins]

I'm scratching an itch at the moment, I just noted this in passing ;)

I'm partial to the 'turn it into a fake test case' approach, its what I
would do if I get to it first.

[slmnhq]

Line 348 in trunk/Lib/test/test_unittest.py has a test case to
specifically test that in the described situation, the test returns an
AttributeError. Should this test be changed so that it passes if the
exception is in fact an ImportError?

   def test_loadTestsFromName__unknown_attr_name(self):
        loader = unittest.TestLoader()

    try:
        loader.loadTestsFromName('unittest.sdasfasfasdf')
    except AttributeError, e:
        self.assertEqual(str(e), "'module' object has no attribute

'sdasfasfasdf'")
else:
self.fail("TestLoader.loadTestsFromName failed to raise
AttributeError")

[slmnhq]

I'm attaching a patch (against trunk) which I think is a step in the
right direction but I could use some feedback.

This patch modifies 'loadTestsFromName()' so that it saves and re-raises
an ImportError.

Further this patch introduces a new unittest
(test_loadTestsFromName__badimport) and slightly modifies two existing
unittests (test_loadTestsFromName__unknown_attr_name,
test_loadTestsFromNames__unknown_attr_name) in test_unittest.py. Also, I
think a second new unittest is needed (test_loadTestsFromNames__badimport).

[slmnhq]

Hope everyone had a good new year's. I've attached an updated patch which adds a new unittest, test_loadTestsFromNames__badimport.

Both the new unittests can use better documentation, hopefully one of you can help me with that.

[voidspace]

Wouldn't this be a backwards incompatible change of tested behaviour though?

[voidspace]

I'm unhappy with a straight change in behaviour because it will break code that is currently catching AttributeError.

A slightly less invasive change would be to raise an AttributeError if the module doesn't exist, otherwise letting the original error propagate.

That means distinguishing between a module not existing and an ImportError raised whilst importing the module. Example code that does this by walking the stack: http://twistedmatrix.com/trac/browser/trunk/twisted/python/reflect.py#L382

In addition we could add a new method that loads a test from name, returning an 'ErrorHolder' if loading the test fails. (A TestCase that reraises the original error when run - test discovery already does this in fact so that a test module failing to load doesn't halt discovery.)