classification
Title: cross platform failure and silly test in doctest
Type: Stage: patch review
Components: Library (Lib) Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: cjw296 Nosy List: benjamin.peterson, cjw296, eric.araujo, georg.brandl, pje, techtonik
Priority: normal Keywords: patch

Created on 2009-08-14 16:18 by cjw296, last changed 2011-09-09 20:13 by pje.

Files
File name Uploaded Description Edit
6703.silly.abspath.test.diff techtonik, 2010-04-01 14:10 patch
Messages (11)
msg91557 - (view) Author: Chris Withers (cjw296) * (Python committer) Date: 2009-08-14 16:18
Hi All,

Line 355 of this code on the 2.6 branch and trunk:

http://svn.python.org/view/python/branches/release26-
maint/Lib/doctest.py?view=annotate
http://svn.python.org/view/python/trunk/Lib/doctest.py?annotate=69012

...contain a check that doesn't work cross platform.

I'd argue that the check is worthless and should be removed.
I'm happy to do this, but is this code tested?
If not, do I need to add a test when I remove those two lines?

cheers,

Chris
msg91567 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-08-14 19:08
How about removing it and seeing if tests fail?
msg92269 - (view) Author: Chris Withers (cjw296) * (Python committer) Date: 2009-09-04 22:09
Hmm, I don't think tests will fail, however, there are cryptic docs for this...

http://docs.python.org/library/doctest.html#basic-api

I don't really get what module_relative is about and I've always run into the non-
cross-platform issue above when passing an absolute path to DocFileSuite while 
forgetting to specify module_relative=False.

The weird/frustrating thing is that the tests behave *just fine* on Windows with 
module_relative being the default of True and the absolute path being specified, so 
I'm not really sure what difference either the module_relative option or the check 
that the path doesn't start with a / mean...

The problem is that while the docs specify that paths must be /-separated, nothing 
in the code enforces this. 

So, with module_relative as True, the default:

- If you generate and pass an absolute path on Windows, it'll work just fine. 

- If you pass a '\\'-separated module-relative path, this will work just fine.
  (You'll end up with base/dir\path\you\supplied, but I doubt `open` will care about 
that.

With module_relative set to False, I'm not really sure what happens...

Anyone?
msg102091 - (view) Author: anatoly techtonik (techtonik) Date: 2010-04-01 14:10
Sure doctests should be crossplatform by default when possible. The patch should fix the behavior for windows. It also cleans the doc and code a bit.

Can you provide some testcases?
msg102092 - (view) Author: anatoly techtonik (techtonik) Date: 2010-04-01 14:12
http://codereview.appspot.com/815042/show
msg122884 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-30 05:33
Patch looks good.  One error and style suggestions on rietveld.
msg122891 - (view) Author: Chris Withers (cjw296) * (Python committer) Date: 2010-11-30 07:45
What this patch doesn't appear to address is that there doesn't appear to be any point to module_relative. Can anyone clarify what the intention of this option is? (it doesn't appear to have any meaningful point from my experience...)
msg143776 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-09-09 17:25
The revision you linked to seems to say that Georg was the one who added this code.
msg143779 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-09-09 17:36
> The revision you linked to seems to say that Georg was the one who
> added this code.

Can you elaborate?  Line 355 in trunk was last edited by "edloper".
msg143782 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-09-09 17:42
I misread the Subversion viewer, it was actually PJE in r45248 (if I read correctly this time :)
msg143799 - (view) Author: PJ Eby (pje) * (Python committer) Date: 2011-09-09 20:13
No, I only consolidated the two copies of the code to a single function, in order to more easily add the PEP 302 support.  The feature was already there.
History
Date User Action Args
2011-09-09 20:13:38pjesetmessages: + msg143799
2011-09-09 17:42:35eric.araujosetnosy: + pje
messages: + msg143782
2011-09-09 17:36:01georg.brandlsetmessages: + msg143779
2011-09-09 17:25:17eric.araujosetnosy: + georg.brandl

messages: + msg143776
versions: + Python 3.3, - Python 3.1
2010-11-30 07:45:40cjw296setmessages: + msg122891
2010-11-30 05:33:49eric.araujosetversions: + Python 3.1, - Python 2.6
nosy: + eric.araujo

messages: + msg122884

components: + Library (Lib)
stage: patch review
2010-04-01 14:12:49techtoniksetmessages: + msg102092
2010-04-01 14:10:03techtoniksetfiles: + 6703.silly.abspath.test.diff

nosy: + techtonik
messages: + msg102091

keywords: + patch
2009-09-04 22:09:47cjw296setmessages: + msg92269
2009-08-14 19:08:30benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg91567
2009-08-14 16:18:16cjw296create