classification
Title: Serious regression in doctest in Py3.1rc1
Type: behavior Stage: resolved
Components: Library (Lib), Tests Versions: Python 3.1
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: r.david.murray Nosy List: amaury.forgeotdarc, dalcinl, r.david.murray, scoder
Priority: high Keywords: patch

Created on 2009-06-04 21:42 by dalcinl, last changed 2009-06-12 15:35 by r.david.murray. This issue is now closed.

Files
File name Uploaded Description Edit
doctest.diff dalcinl, 2009-06-04 21:42
issue6195.patch r.david.murray, 2009-06-12 12:35
Messages (10)
msg88907 - (view) Author: Lisandro Dalcin (dalcinl) Date: 2009-06-04 21:42
When doctests are written in docstrings from C extension modules,
'doctest' reads the binary extension module file.

The attached one-line patch seems to fix the problem, it is in fact very
similar to patch for issue4050 related to 'inspect'.
msg88916 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-05 01:53
Here's a test case which fails with the existing code and passes with
the patch applied.

However, with the patch applied some of the other doctest tests fail
with an off-by-one error in the source line number output.  I'd like to
understand that difference before committing the patch, but haven't
figured it out yet.
msg88935 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2009-06-05 11:04
Patches (fix+test) are good.
msg88984 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-06 00:17
The fix is not in fact correct.  Without the fix, source code is found
that is skipped with the fix in place.  This may mean that the fix for
issue4050 is also in error.

The object found by inspect.getfile when it isn't an a .so is of the form:

   <doctest test.test_doctest.test_DocTestRunner.basics[3]>

So I think we need some way to determine whether or not what is returned
by getfile is binary data or not.
msg89000 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-06 13:10
It turns out that doctest patches linecache.getlines (which is not, by
the way, a public interface of linecache according to the docs and the
__all__ string) to retrieve the doctest source code.  The special
filename it uses to trigger this is not returned by getsourcefile but is
returned by getfile.

So the issue4050 fix is not impacted, but the fix for this bug is not so
straightforward.
msg89003 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-06 13:54
OK, here is a revised patch that passes all tests, including the new one.
msg89149 - (view) Author: Lisandro Dalcin (dalcinl) Date: 2009-06-09 15:17
I've tested latest David's patch against Cython test suite (doctests
living in extension modules), and all is working as expected.
msg89256 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-11 17:53
Unfortunately we're not out of the woods yet.  test_zipimport_support
has a failure with the patch applied.  I will investigate when I get a
chance, but I'm beginning to wonder if the doctest getsouce code needs
some refactoring.
msg89274 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-12 12:35
The problem turned out to be a cut and paste error in my patch.  I'm
uploading the corrected patch, which passes all tests.
msg89280 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-12 15:35
Fixed in r73389.

I am wondering if the monkeypatching of linecache that doctest does
could be done away with, but I'm not sure whether it might get used
somehow in debug mode (since the comment refers to that).  On the other
hand, if it does get used, the same binary-read issue may exist in that
hypothetical code path.
History
Date User Action Args
2009-06-12 15:35:44r.david.murraysetstatus: open -> closed
resolution: fixed
messages: + msg89280

stage: patch review -> resolved
2009-06-12 12:35:54r.david.murraysetfiles: - issue6195.patch
2009-06-12 12:35:48r.david.murraysetfiles: - doctest-test.diff
2009-06-12 12:35:37r.david.murraysetfiles: + issue6195.patch

messages: + msg89274
2009-06-11 17:53:38r.david.murraysetmessages: + msg89256
2009-06-09 15:17:09dalcinlsetmessages: + msg89149
2009-06-06 13:54:06r.david.murraysetfiles: + issue6195.patch

messages: + msg89003
stage: needs patch -> patch review
2009-06-06 13:10:19r.david.murraysetmessages: + msg89000
2009-06-06 00:17:40r.david.murraysetresolution: accepted -> (no value)
messages: + msg88984
stage: patch review -> needs patch
2009-06-05 11:04:34amaury.forgeotdarcsetresolution: accepted

messages: + msg88935
nosy: + amaury.forgeotdarc
2009-06-05 01:53:30r.david.murraysetfiles: + doctest-test.diff
type: behavior
messages: + msg88916

stage: test needed -> patch review
2009-06-05 00:41:50r.david.murraysetpriority: high
assignee: r.david.murray

nosy: + r.david.murray
stage: test needed
2009-06-04 21:42:58dalcinlcreate