classification
Title: doctest 'fancy diff' formats incorrectly strip trailing whitespace
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.8, Python 3.7, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: orsenthil Nosy List: CuriousLearner, jairotrad, miss-islington, ned.deily, orsenthil, pablogsal, r.david.murray, tim.peters, vstinner
Priority: normal Keywords: easy, patch

Created on 2015-07-29 01:26 by r.david.murray, last changed 2019-01-11 13:58 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
doctest_fancy_diff_trailing_whitespace.diff r.david.murray, 2015-07-29 01:26 review
issue24746.patch jairotrad, 2015-08-17 00:00 review
Pull Requests
URL Status Linked Edit
PR 10639 merged CuriousLearner, 2018-11-21 20:02
PR 10639 merged CuriousLearner, 2018-11-21 20:02
PR 11476 merged miss-islington, 2019-01-09 13:39
PR 11476 merged miss-islington, 2019-01-09 13:39
PR 11476 merged miss-islington, 2019-01-09 13:39
PR 11477 merged miss-islington, 2019-01-09 13:39
PR 11477 merged miss-islington, 2019-01-09 13:39
PR 11477 merged miss-islington, 2019-01-09 13:39
PR 11482 merged CuriousLearner, 2019-01-09 18:43
PR 11482 merged CuriousLearner, 2019-01-09 18:43
PR 11496 closed vstinner, 2019-01-10 07:51
PR 11496 closed vstinner, 2019-01-10 07:51
PR 11496 closed vstinner, 2019-01-10 07:51
PR 11498 closed vstinner, 2019-01-10 07:54
PR 11498 closed vstinner, 2019-01-10 07:54
PR 11498 closed vstinner, 2019-01-10 07:54
PR 11499 closed vstinner, 2019-01-10 07:55
PR 11499 closed vstinner, 2019-01-10 07:55
PR 11499 closed vstinner, 2019-01-10 07:55
PR 11501 merged pablogsal, 2019-01-10 11:21
PR 11501 merged pablogsal, 2019-01-10 11:21
PR 11501 merged pablogsal, 2019-01-10 11:21
PR 11505 merged miss-islington, 2019-01-10 15:37
PR 11505 merged miss-islington, 2019-01-10 15:37
PR 11505 merged miss-islington, 2019-01-10 15:37
PR 11509 merged orsenthil, 2019-01-10 17:35
PR 11509 merged orsenthil, 2019-01-10 17:35
PR 11509 merged orsenthil, 2019-01-10 17:35
PR 11512 merged orsenthil, 2019-01-10 18:05
PR 11512 merged orsenthil, 2019-01-10 18:05
PR 11512 merged orsenthil, 2019-01-10 18:05
PR 11512 merged orsenthil, 2019-01-10 18:05
Messages (26)
msg247552 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-07-29 01:26
I got a doctest failure that when I wrote the output to a file showed two exactly identical lines reported as being different.  Turning off the fancy diff, I could see trailing whitespace on one of the lines.  It turns out that when a fancy diff is requested, doctest explicitly goes through and strips trailing whitespace from the diff lines returned by difflib.  This seems to me to be obviously incorrect.  There is no clue in the changelog why this was done...this goes back to a massive refactoring of doctest that was done for python 2.4, and the fancy diff was introduced at that point, complete with this strange behavior.

I tried to write a test for this but couldn't get it working in the time I was willing to devote to this (I've switched to NDIFF format, which shows the whitespace error even when the actual whitespace is stripped).  Perhaps testing this via doctest isn't the best idea anyway, since it will be far to easy for the trailing whitespace in the test to get accidentally stripped.  I've attached my work as a diff for reference if someone wants to work on this.
msg248695 - (view) Author: Jairo Trad (jairotrad) * Date: 2015-08-17 00:00
I have patched this as explained by David. Also the tests are working. Another test broke because a missing trailing space. I fixed that too.

This is my first patch :D
msg248701 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-08-17 01:12
Unfortunately I was reminded a few days ago that there is a commit hook that prevents patches containing trailing whitespace from being committed to the repository.  So using doctest to test this isn't going to work.

The alternatives are to write a unit test, or to figure out how to write the doctest such there is no trailing whitespace...which would mean capturing the output of doctest.DoctTestRunner instead of letting it go the console, and then comparing it to an explicitly embedded string.
msg248726 - (view) Author: Jairo Trad (jairotrad) * Date: 2015-08-17 14:01
I can write a unittest for this, where should I write it? a new test file?
msg248727 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-08-17 14:04
I believe you can put it in the test_doctest file, and call it using the appropriate runner from test.support from the test_main function.  I haven't tried it though.
msg330214 - (view) Author: Sanyam Khurana (CuriousLearner) * (Python triager) Date: 2018-11-21 20:05
> Unfortunately I was reminded a few days ago that there is a commit hook that prevents patches containing trailing whitespace from being committed to the repository.

Is this still the case?

I tried to figure out writing the test case, but I can't wrap my head around showing the trailing whitespace without putting it anywhere in the code.

Also, can you have a look at the PR, please?
msg330220 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2018-11-21 22:16
To include trailing whitespace on a line in a doctest, _don't_ use raw strings.  Use a regular string, and include add a (one or more) trailing \x20 instead of a space (for example).  For example:

r"""
>>> print("a ")
a 
"""

where there's a space at the end of the output line is no good.  It's visually impossible to tell what's intended, and the commit hook should reject it.  But this works fine (a regular string and an escape code):

"""
>>> print("a ")
a\x20
"""
msg330240 - (view) Author: Sanyam Khurana (CuriousLearner) * (Python triager) Date: 2018-11-22 10:04
Thanks for your suggestion, Tim.

I've fixed the patch and it is ready for review at https://github.com/python/cpython/pull/10639
msg333319 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2019-01-09 13:38
New changeset cbb16459934eaf29c7c7d362939cd05550b2f21f by Senthil Kumaran (Sanyam Khurana) in branch 'master':
bpo-24746: Avoid stripping trailing whitespace in doctest fancy diff (#10639)
https://github.com/python/cpython/commit/cbb16459934eaf29c7c7d362939cd05550b2f21f
msg333320 - (view) Author: miss-islington (miss-islington) Date: 2019-01-09 13:56
New changeset 53cf5f084b01cb16630361be5377047c068d2b44 by Miss Islington (bot) in branch '3.7':
bpo-24746: Avoid stripping trailing whitespace in doctest fancy diff (GH-10639)
https://github.com/python/cpython/commit/53cf5f084b01cb16630361be5377047c068d2b44
msg333322 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2019-01-09 14:46
New changeset 5d9ae8b9df8371dd65514e0d60b561fd37056986 by Senthil Kumaran (Miss Islington (bot)) in branch '3.6':
bpo-24746: Avoid stripping trailing whitespace in doctest fancy diff (GH-10639) (#11477)
https://github.com/python/cpython/commit/5d9ae8b9df8371dd65514e0d60b561fd37056986
msg333341 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2019-01-09 19:03
New changeset 02e33d9567aa4bd612f9f08053acbfd5e68480d0 by Senthil Kumaran (Sanyam Khurana) in branch '2.7':
[2.7] bpo-24746: Avoid stripping trailing whitespace in doctest fancy diff (#11482)
https://github.com/python/cpython/commit/02e33d9567aa4bd612f9f08053acbfd5e68480d0
msg333360 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-01-10 01:29
test_doctest starts to fail on AMD64 Windows8.1 Refleaks 2.7:

https://buildbot.python.org/all/#/builders/33/builds/471

It may be related to:

New changeset 02e33d9567aa4bd612f9f08053acbfd5e68480d0 by Senthil Kumaran (Sanyam Khurana) in branch '2.7':
[2.7] bpo-24746: Avoid stripping trailing whitespace in doctest fancy diff (#11482)
https://github.com/python/cpython/commit/02e33d9567aa4bd612f9f08053acbfd5e68480d0
msg333361 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-01-10 01:31
Yeah, I confirm that it's a regression caused by commit 02e33d9567aa4bd612f9f08053acbfd5e68480d0:

"./python -m test test_doctest test_doctest" fails after this commit, but pass before this commit.
msg333362 - (view) Author: Sanyam Khurana (CuriousLearner) * (Python triager) Date: 2019-01-10 06:15
Hi Victor,

Senthil reverted the backport commit on 2.7 branch.

Is there a way I can test reference leaks in CPython? Is it using valgrind or some other tool? Can you please point me to that?

I can then see if I can fix this on 2.7

Thanks!
msg333363 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-01-10 06:33
In short, this buildbot runs "./python -m test -R 3:3 test_doctest". But
here the bug is just that running the test twice in a row fails.
msg333367 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-01-10 07:49
Oh. The 3.6, 3.7 and master changes broke the Refleaks buildbots... By the way, why was such bugfix merged into the 3.6 branch which is now in security-only mode?

Please fix this bug ASAP, or I will revert the change.
msg333372 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-01-10 11:22
I made https://github.com/python/cpython/pull/11501 to fix this problem. After my patch:

❯ ./python.exe -m test test_doctest test_doctest
Run tests sequentially
0:00:00 load avg: 1.82 [1/2] test_doctest
0:00:02 load avg: 1.82 [2/2] test_doctest

== Tests result: SUCCESS ==

All 2 tests OK.

Total duration: 5 sec 286 ms
Tests result: SUCCESS
msg333382 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2019-01-10 14:29
New changeset c5dc60ea858b8ccf78e8d26db81c307a8f9b2314 by Senthil Kumaran (Pablo Galindo) in branch 'master':
bpo-24746: Fix doctest failures when running the testsuite with -R (#11501)
https://github.com/python/cpython/commit/c5dc60ea858b8ccf78e8d26db81c307a8f9b2314
msg333388 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2019-01-10 15:45
Hi All,

It was my mistake to merge this in into 3.6, I didn't realize 3.6 was in bugfix mode now. Also I went by the versions (previously) set in this bpo issue.

For the regression caused in refleaks, I think, Pablo's patch will fix it, and I am verifying it here - https://buildbot.python.org/all/#/builders/1/builds/468

This will be backported to 3.7 ( I notice viktor triggered already: https://github.com/python/cpython/pull/11505) and I will cherrpick this to 2.7.

Finally, since the the port to 3.6 was a mistake, I will revert that.

This will make sure that bug is fixed all the supported versions, target branches tests are successful and 3.6 remains unaffected.
msg333390 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-01-10 15:57
> Finally, since the the port to 3.6 was a mistake, I will revert that.

I wrote PR 11499 but I closed it because PR #11501 has been merged, but yeah, maybe a revert is the best option for 3.6.

No problem, the status change of the 3.6 branch is very new, and not everybody got the memo ;-)
https://devguide.python.org/#status-of-python-branches

See this discussion:
https://discuss.python.org/t/remove-needs-backport-to-3-6-label/563
msg333392 - (view) Author: miss-islington (miss-islington) Date: 2019-01-10 16:02
New changeset 1cbd17c6987afc48c16caa7ccc7d19b01fbd39f2 by Miss Islington (bot) in branch '3.7':
bpo-24746: Fix doctest failures when running the testsuite with -R (GH-11501)
https://github.com/python/cpython/commit/1cbd17c6987afc48c16caa7ccc7d19b01fbd39f2
msg333415 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2019-01-10 18:56
New changeset d09e8cecf214b1de457feae01860f5592f912a8e by Ned Deily (Senthil Kumaran) in branch '3.6':
Revert "bpo-24746: Avoid stripping trailing whitespace in doctest fancy diff (GH-10639) (GH-11477)" (GH-11509)
https://github.com/python/cpython/commit/d09e8cecf214b1de457feae01860f5592f912a8e
msg333417 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2019-01-10 20:55
New changeset 0167c08163f44f4a033497102244bbb6150f606b by Senthil Kumaran in branch '2.7':
bpo-24746: Fix doctest failures when running the testsuite with -R (#11501) (#11512)
https://github.com/python/cpython/commit/0167c08163f44f4a033497102244bbb6150f606b
msg333431 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2019-01-11 01:18
All changes related to this issue are done.

Thanks for your contribution and engagement everyone!
msg333468 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-01-11 13:58
All Refleaks buildots are back to green, thanks ;-)
History
Date User Action Args
2019-01-11 13:58:34vstinnersetmessages: + msg333468
2019-01-11 01:18:33orsenthilsetstatus: open -> closed
resolution: fixed
messages: + msg333431

stage: patch review -> resolved
2019-01-10 20:55:16orsenthilsetmessages: + msg333417
2019-01-10 18:56:13ned.deilysetnosy: + ned.deily
messages: + msg333415
2019-01-10 18:13:16orsenthilsetversions: + Python 2.7
2019-01-10 18:05:35orsenthilsetpull_requests: + pull_request11069
2019-01-10 18:05:26orsenthilsetpull_requests: + pull_request11070
2019-01-10 18:05:16orsenthilsetpull_requests: + pull_request11068
2019-01-10 18:05:05orsenthilsetpull_requests: + pull_request11067
2019-01-10 17:36:09orsenthilsetpull_requests: + pull_request11060
2019-01-10 17:35:55orsenthilsetpull_requests: + pull_request11059
2019-01-10 17:35:39orsenthilsetpull_requests: + pull_request11058
2019-01-10 16:02:31miss-islingtonsetmessages: + msg333392
2019-01-10 15:57:31vstinnersetmessages: + msg333390
2019-01-10 15:45:01orsenthilsetassignee: CuriousLearner -> orsenthil
messages: + msg333388
2019-01-10 15:38:04miss-islingtonsetpull_requests: + pull_request11046
2019-01-10 15:37:54miss-islingtonsetpull_requests: + pull_request11045
2019-01-10 15:37:40miss-islingtonsetpull_requests: + pull_request11044
2019-01-10 14:29:43orsenthilsetmessages: + msg333382
2019-01-10 11:22:14pablogsalsetnosy: + pablogsal
messages: + msg333372
2019-01-10 11:22:02pablogsalsetpull_requests: + pull_request11032
2019-01-10 11:21:50pablogsalsetpull_requests: + pull_request11031
2019-01-10 11:21:36pablogsalsetpull_requests: + pull_request11030
2019-01-10 07:56:09vstinnersetversions: - Python 3.4, Python 3.5, Python 3.6
2019-01-10 07:55:36vstinnersetpull_requests: + pull_request11028
2019-01-10 07:55:23vstinnersetpull_requests: + pull_request11027
2019-01-10 07:55:11vstinnersetpull_requests: + pull_request11026
2019-01-10 07:54:52vstinnersetpull_requests: + pull_request11025
2019-01-10 07:54:38vstinnersetpull_requests: + pull_request11024
2019-01-10 07:54:23vstinnersetpull_requests: + pull_request11023
2019-01-10 07:52:18vstinnersetpull_requests: + pull_request11022
2019-01-10 07:52:03vstinnersetpull_requests: + pull_request11021
2019-01-10 07:51:49vstinnersetpull_requests: + pull_request11020
2019-01-10 07:49:21vstinnersetmessages: + msg333367
2019-01-10 06:33:36vstinnersetmessages: + msg333363
2019-01-10 06:15:16CuriousLearnersetmessages: + msg333362
2019-01-10 01:31:37vstinnersetmessages: + msg333361
2019-01-10 01:29:45vstinnersetnosy: + vstinner
messages: + msg333360
2019-01-09 19:03:11orsenthilsetmessages: + msg333341
2019-01-09 18:43:56CuriousLearnersetpull_requests: + pull_request11000
2019-01-09 18:43:47CuriousLearnersetpull_requests: + pull_request10999
2019-01-09 14:46:32orsenthilsetmessages: + msg333322
2019-01-09 13:56:51miss-islingtonsetnosy: + miss-islington
messages: + msg333320
2019-01-09 13:39:54miss-islingtonsetpull_requests: + pull_request10986
2019-01-09 13:39:47miss-islingtonsetstage: test needed -> patch review
pull_requests: + pull_request10984
2019-01-09 13:39:40miss-islingtonsetstage: test needed -> test needed
pull_requests: + pull_request10983
2019-01-09 13:39:32miss-islingtonsetpull_requests: + pull_request10985
2019-01-09 13:39:20miss-islingtonsetstage: test needed -> test needed
pull_requests: + pull_request10982
2019-01-09 13:39:11miss-islingtonsetstage: test needed -> test needed
pull_requests: + pull_request10981
2019-01-09 13:38:46orsenthilsetnosy: + orsenthil
messages: + msg333319
2018-11-22 12:47:36CuriousLearnersetassignee: CuriousLearner
2018-11-22 10:04:06CuriousLearnersetmessages: + msg330240
2018-11-21 22:16:38tim.peterssetnosy: + tim.peters
messages: + msg330220
2018-11-21 20:05:30CuriousLearnersetversions: + Python 3.7, Python 3.8
nosy: + CuriousLearner

messages: + msg330214

stage: patch review -> test needed
2018-11-21 20:02:29CuriousLearnersetstage: test needed -> patch review
pull_requests: + pull_request9887
2018-11-21 20:02:26CuriousLearnersetstage: test needed -> test needed
pull_requests: + pull_request9886
2015-08-17 14:04:46r.david.murraysetmessages: + msg248727
2015-08-17 14:01:23jairotradsetmessages: + msg248726
2015-08-17 01:12:36r.david.murraysetmessages: + msg248701
2015-08-17 00:00:23jairotradsetfiles: + issue24746.patch
nosy: + jairotrad
messages: + msg248695

2015-07-29 01:26:52r.david.murraycreate