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

#19217: Calling assertEquals for moderately long list takes too long

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 5 months ago by jbzdak
Modified:
3 years, 1 month ago
Reviewers:
pitrou, storchaka, ezio.melotti
CC:
gregory.p.smith, pmoore, AntoinePitrou, haypo, rbcollins, tim.golden, ezio.melotti, Michael Foord, Zach Ware, storchaka, jbzdak_gmail.com, steve.dower, ankur, Elena.Oat, nzakharenko_gmail.com, ankur, punchagan_muse-amuse.in, levkivskyi, chris_atlee.ca, Eric Lafontaine, eamanu, xtreak
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Total comments: 5

Patch Set 4 #

Total comments: 7

Patch Set 5 #

Total comments: 7

Patch Set 6 #

Patch Set 7 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/unittest/case.py View 1 2 3 4 5 6 1 chunk +4 lines, -3 lines 0 comments Download
Lib/unittest/test/test_assertions.py View 1 2 3 4 5 6 1 chunk +2 lines, -2 lines 0 comments Download
Lib/unittest/test/test_case.py View 1 2 3 4 5 6 1 chunk +4 lines, -2 lines 0 comments Download

Messages

Total messages: 5
pitrou_free.fr
Elena, thank you for the patch. I have added some comments below. http://bugs.python.org/review/19217/diff/12644/Lib/unittest/case.py File Lib/unittest/case.py ...
5 years, 8 months ago #1
storchaka_gmail.com
http://bugs.python.org/review/19217/diff/12646/Lib/unittest/case.py File Lib/unittest/case.py (right): http://bugs.python.org/review/19217/diff/12646/Lib/unittest/case.py#newcode358 Lib/unittest/case.py:358: _seqDiffThreshold = 32 # used for sequences and containers ...
5 years, 8 months ago #2
ezio.melotti
http://bugs.python.org/review/19217/diff/12646/Lib/unittest/case.py File Lib/unittest/case.py (right): http://bugs.python.org/review/19217/diff/12646/Lib/unittest/case.py#newcode358 Lib/unittest/case.py:358: _seqDiffThreshold = 32 # used for sequences and containers ...
5 years, 7 months ago #3
ezio.melotti
I added a few comments, but then noticed that the patch doesn't seem 100% complete ...
5 years, 7 months ago #4
storchaka_gmail.com
5 years, 7 months ago #5
http://bugs.python.org/review/19217/diff/12673/Lib/unittest/case.py
File Lib/unittest/case.py (right):

http://bugs.python.org/review/19217/diff/12673/Lib/unittest/case.py#newcode23
Lib/unittest/case.py:23: DIFF_OMITTED = ('*** Diff is %s characters long.  '
On 2014/08/10 10:50:16, ezio.melotti wrote:
> I don't see the reason to keep this here, but since you added another message,
> either they are both here, or both in the function.

I left it here only because it is used in tests.

http://bugs.python.org/review/19217/diff/12673/Lib/unittest/case.py#newcode976
Lib/unittest/case.py:976: pprint.pformat(seq2).splitlines())
On 2014/08/10 10:50:16, ezio.melotti wrote:
> Have you checked that this is fast enough in case there are lot of lines? 

This has O(N) complexity and is not a matter of this issue.

Note that all this code is executed only when test is failed. It is fast enough
for error reporting (it is only by constant multiplier slower than repr() which
is calculated above).

http://bugs.python.org/review/19217/diff/12673/Lib/unittest/case.py#newcode1002
Lib/unittest/case.py:1002: def _truncateMessage(self, message, diff):
On 2014/08/10 10:50:16, ezio.melotti wrote:
> This is now used only by assertCountEqual, and that should probably use
> _truncateDiffs too.

I doubt assertCountEqual can utilize _truncateDiffs.
Sign in to reply to this message.

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