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

#7559: TestLoader.loadTestsFromName swallows import errors

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 6 months ago by robertc
Modified:
3 years, 8 months ago
Reviewers:
chris.jerdonek, rdmurray
CC:
brett.cannon, rbcollins, v.ladeuil+bugs-python_free.fr, mcepl, ielectric_gmail.com, ezio.melotti, r.david.murray, Michael Foord, salman.haq_gmail.com, cjerdonek, BreamoreBoy, Julian, devnull_psf.upfronthosting.co.za, eric.snow, alex.garel_gmail.com, berkerpeksag, Zach Ware, Tim.Graham
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Patch Set 4 #

Patch Set 5 #

Patch Set 6 #

Patch Set 7 #

Patch Set 8 #

Patch Set 9 #

Total comments: 1

Patch Set 10 #

Total comments: 12

Patch Set 11 #

Patch Set 12 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/unittest.rst View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +6 lines, -0 lines 0 comments Download
Lib/unittest/loader.py View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +31 lines, -4 lines 0 comments Download
Lib/unittest/test/test_loader.py View 1 2 3 4 5 6 7 8 9 10 11 15 chunks +150 lines, -101 lines 0 comments Download
Misc/NEWS View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 3
cjerdonek
http://bugs.python.org/review/7559/diff/5057/Lib/unittest/loader.py File Lib/unittest/loader.py (right): http://bugs.python.org/review/7559/diff/5057/Lib/unittest/loader.py#newcode101 Lib/unittest/loader.py:101: if e.name != aname and not str(e).endswith('not a package'): ...
6 years ago #1
r.david.murray
http://bugs.python.org/review/7559/diff/13111/Doc/library/unittest.rst File Doc/library/unittest.rst (right): http://bugs.python.org/review/7559/diff/13111/Doc/library/unittest.rst#newcode1638 Doc/library/unittest.rst:1638: Python :mod:`unittest` package. I"m still confused by the language ...
3 years, 8 months ago #2
rbcollins
3 years, 8 months ago #3
http://bugs.python.org/review/7559/diff/13111/Doc/library/unittest.rst
File Doc/library/unittest.rst (right):

http://bugs.python.org/review/7559/diff/13111/Doc/library/unittest.rst#newcod...
Doc/library/unittest.rst:1638: Python :mod:`unittest` package.
On 2014/10/24 00:48:38, r.david.murray wrote:
> I"m still confused by the language about "non-fatal error" and what exactly
gets
> accumulated into 'errors'.  I think you could replace everything after the
first
> sentence with "These errors are included in the errors accumulated by
> self.errors".  The last sentence is more appropriate for the what's new
porting
> notes.

I'm not sure what you mean by that last sentence. I've deleted it for now.

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

http://bugs.python.org/review/7559/diff/13111/Lib/unittest/loader.py#newcode147
Lib/unittest/loader.py:147: # Nothing succeeded, signal error immediately.
On 2014/10/24 00:48:38, r.david.murray wrote:
> "signal error immediately" sounds like an error will be raised at this point,
> but you are still returning a synthetic test here.  How about "Nothing
> succeeded, report the last error."?

Done.

http://bugs.python.org/review/7559/diff/13111/Lib/unittest/loader.py#newcode157
Lib/unittest/loader.py:157: # If the current object is a package we presume that
the
On 2014/10/24 00:48:38, r.david.murray wrote:
> Given the next comment, this sentence appears redundant and should be deleted.

Done.

http://bugs.python.org/review/7559/diff/13111/Lib/unittest/loader.py#newcode174
Lib/unittest/loader.py:174: return error_case
On 2014/10/24 00:48:38, r.david.murray wrote:
> Looks like you could reverse the sense of the if and eliminate two lines of
> repeated code.

We could, but then reading the code will require unpacking a nasty nasty boolean
expression. append-and-return is small enough that I think this is a better
tradeoff.

http://bugs.python.org/review/7559/diff/13111/Lib/unittest/test/test_loader.py
File Lib/unittest/test/test_loader.py (right):

http://bugs.python.org/review/7559/diff/13111/Lib/unittest/test/test_loader.p...
Lib/unittest/test/test_loader.py:435: # What happens when the module is found,
but the attribute can't?
On 2014/10/24 00:48:38, r.david.murray wrote:
> isn't
hah, copy-paste itis. That can't is from bad comments in the file already.
Fixing them up.

http://bugs.python.org/review/7559/diff/13111/Lib/unittest/test/test_loader.p...
Lib/unittest/test/test_loader.py:508: AttributeError, expected_regex,
getattr(test, 'abc () //'))
On 2014/10/24 00:48:38, r.david.murray wrote:
> Isn't this a duplicate test, except for the comment?

Kindof - one is 'relative' and one 'absolute', but I'm not sure that the
difference is relevant in this case, I'd like to clean these tests up, but thats
a different change.
Sign in to reply to this message.

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