Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(51710)

#17457: Unittest discover fails with namespace packages and builtin modules

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 7 months ago by pcmanticore
Modified:
5 years, 10 months ago
Reviewers:
ezio.melotti, ericsnowcurrently
CC:
eric.smith, ezio.melotti, Michael Foord, Claudiu.Popa, devnull_psf.upfronthosting.co.za, eric.snow
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 2

Patch Set 3 #

Total comments: 2

Patch Set 4 #

Patch Set 5 #

Total comments: 2

Patch Set 6 #

Patch Set 7 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/unittest.rst View 1 2 3 4 5 6 2 chunks +8 lines, -1 line 0 comments Download

Messages

Total messages: 3
ezio.melotti
http://bugs.python.org/review/17457/diff/7693/Lib/unittest/loader.py File Lib/unittest/loader.py (right): http://bugs.python.org/review/17457/diff/7693/Lib/unittest/loader.py#newcode231 Lib/unittest/loader.py:231: raise TypeError('Can not use builtin modules as dotted module ...
5 years, 11 months ago #1
eric.snow
I haven't looked at what this patch actually does, so I don't have a feedback ...
5 years, 11 months ago #2
eric.snow
5 years, 11 months ago #3
Thanks for doing that, especially now that PEP 451 has landed. :)

http://bugs.python.org/review/17457/diff/10040/Lib/unittest/loader.py
File Lib/unittest/loader.py (right):

http://bugs.python.org/review/17457/diff/10040/Lib/unittest/loader.py#newcode223
Lib/unittest/loader.py:223: spec = the_module.__spec__
There is no guarantee that the_module has a __spec__ attribute (if the module
replaced itself in sys.modules).  So you might want to handle AttributeError
here appropriately.

http://bugs.python.org/review/17457/diff/10040/Lib/unittest/loader.py#newcode238
Lib/unittest/loader.py:238: elif spec.name in sys.builtin_module_names:
This should probably stay the_module.__name__, since there is no guarantee that
the module's published name will actually match the name in the spec.  (In
practice it will match though.)
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+