[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Chris Jerdonek added the comment:

Thanks a lot, David.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Roundup Robot]

Roundup Robot added the comment:

New changeset c5fc49bc7a5f by R David Murray in branch '2.7':
#14649: add sample files omitted from previous checkin.
http://hg.python.org/cpython/rev/c5fc49bc7a5f

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[R. David Murray]

R. David Murray added the comment:

Thanks, Chris.

--
resolution:  -> fixed
stage: patch review -> committed/rejected
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Roundup Robot]

Roundup Robot added the comment:

New changeset d012f645b8b2 by R David Murray in branch '3.2':
#14649: clarify DocTestSuite error when there are no docstrings.
http://hg.python.org/cpython/rev/d012f645b8b2

New changeset 6544fc92b528 by R David Murray in branch 'default':
Merge #14649: clarify DocTestSuite error when there are no docstrings.
http://hg.python.org/cpython/rev/6544fc92b528

New changeset b48ef168d8c5 by R David Murray in branch '2.7':
#14649: clarify DocTestSuite error when there are no docstrings.
http://hg.python.org/cpython/rev/b48ef168d8c5

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[R. David Murray]

R. David Murray added the comment:

I have separate working directories for 3.2 vs 3.3.  If you have a single 
working directory, the files would continue to be present even without the add. 
 The add is definitely needed for a correctly committed changeset.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Chris Jerdonek added the comment:

What workflow/command are you using to apply and then merge the patch?  I 
didn't need to run "hg add" at any point to go from 3.2 to default, and run the 
tests, etc.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[R. David Murray]

R. David Murray added the comment:

Ah, my mistake.  I forgot to do the hg add for the new files.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Chris Jerdonek added the comment:

I tried applying the latest patch (patch #6) to 3.2, merging to default, and 
then running all tests in default, and the tests seem to all pass for me (in 
both default and 3.2).  Also, the "diff" for the change in default after that 
process seems to be identical to the uploaded patch.

Is it possible that you applied the wrong patch at some point in the process?  
What test failure were you getting?

Can you try applying the patch as is to the default branch (i.e. without going 
through 3.2) and see if you still get test failures?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Chris Jerdonek added the comment:

I prepared the patch against 3.3 (default), so I wouldn't have expected it to 
work against 3.2 without further changes.  The behavior in 3.3 after merging is 
also surprising given that it worked in 3.2 (and for me with direct 
application).

I will look into it and report back -- providing 3.2 and a "delta" patch, if 
necessary.  I just wanted to be sure you were okay with the patch for the 
default branch before doing that extra work.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[R. David Murray]

R. David Murray added the comment:

OK, 3.2 passes now, but when I merge to 3.3 I get failures.  
test_zipimport_support doesn't work on 2.7 either, but there we could just not 
backport the tests.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Chris Jerdonek added the comment:

I updated the patch to fix the test_zipimport_support tests.  All tests now 
pass.  The only changes were to Lib/test/test_zipimport_support.py.

I'll make a note about test_pyclbr and test_zipimport_support when I create the 
issue to move the supporting doctest files into a subdirectory.  Thanks again 
for the catch, David.

--
Added file: http://bugs.python.org/file27040/issue-14649-6.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Chris Jerdonek added the comment:

> With your patch 5 applied, test_zipimport_support fails.

You're right.  It looks like test_zipimport_support is tightly coupled to 
test_doctest.  For example--

http://hg.python.org/cpython/file/786d9516663e/Lib/test/test_zipimport_support.py#l94

My apologies for not running the full test suite before uploading those 
patches.  I'll take a closer look.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[R. David Murray]

R. David Murray added the comment:

With your patch 5 applied, test_zipimport_support fails.  I took a quick look 
at this and it looks like it is because we've added a dependency on an external 
data file to the test for DocTestSuite.

Note that with patch 4 applied, test_pyclbr fails...I found that out by 
accident.  Not sure why test_zipimport_support didn't fail with that one.  But 
regardless we'll have to deal with pyclbr if/when we isolate the doctest test 
files.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Chris Jerdonek added the comment:

Updating the patch after discussing with David on IRC.

The two new files are now added to Lib/test instead of to a subdirectory of 
Lib/test.  Moving the doctest files to a subdirectory can be discussed and 
possibly addressed as part of a separate issue.

--
Added file: http://bugs.python.org/file27027/issue-14649-5.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Chris Jerdonek added the comment:

Small tweak to a code comment in the patch (`tests` is not itself a 
unittest.TestSuite instance).

--
Added file: http://bugs.python.org/file27000/issue-14649-4.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Chris Jerdonek added the comment:

> Personally I prefer to have the test case create the file(s) used in the test 
> dynamically, writing them to the temporary working directory.

I prefer that too, but when I approached this issue, I found that test_doctest 
doesn't use unittest.  It uses only doctest tests to test itself, and it uses 
checked-in files to test things like DocTestSuite and DocFileSuite.  For 
example, test_DocFileSuite has this:

>>> suite = doctest.DocFileSuite('test_doctest.txt',
...  'test_doctest2.txt',
...  'test_doctest4.txt')

(Hence the eight supporting files I mentioned in my previous comment, of which 
these are three.)

Should I break the current way of doing things for this module and introduce 
unittest and the first dynamically created files?

> We have in the past put notes in the doc that say "this may change in the 
> future", though that is usually about desired enhancements.

That's interesting.  It's like a "Will change in version ..." counterpart to 
"Changed in version "  Come to think of it, that could be a useful practice 
in general because it would let people future-proof their code more easily and 
give them incentives to upgrade.

To simplify things for the purposes of this issue, we can discuss adding such 
wording to earlier versions only if and when we actually change the future 
behavior.  I can create a separate issue to discuss changing future behavior 
once this issue is complete.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[R. David Murray]

R. David Murray added the comment:

Personally I prefer to have the test case create the file(s) used in the test 
dynamically, writing them to the temporary working directory.  Since these are 
Python modules, you could use the helpers from script_helpers for this.

Otherwise I think your patch is OK.  I don't think we have a doc convention for 
"unfortunate implementation detail" :(  We have in the past put notes in the 
doc that say "this may change in the future", though that is usually about 
desired enhancements.  But perhaps it could apply here as well.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Changes by Chris Jerdonek :


Added file: http://bugs.python.org/file26991/issue-14649-3.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Chris Jerdonek added the comment:

Here is an updated patch for review (just for the default branch for now).

The main change from the previous patch is that the ValueError exception is now 
documented.

A few additional comments/questions:

Is there a better way to signify in the documentation that certain behavior is 
an unfixable bug as opposed to desired behavior?  Would that be appropriate 
here?

I created a new folder inside Lib/test for supporting test files specific to 
doctest.  In addition to the two new files this patch adds, I would also like 
to move the eight existing support files into this new directory:

doctest_aliases.py
test_doctest.txt
test_doctest3.txt
sample_doctest.py
test_doctest2.py
test_doctest4.txt
test_doctest.py
test_doctest2.txt

Can that be done as a separate change after the patch for this issue, or should 
it be done before?

Assuming people are okay with keeping this ValueError behavior the same in the 
maintenance releases (i.e. not handling this as a bug fix), would people be 
open to changing the behavior in future versions, or should we keep the 
behavior the same in future versions as well?

--
Added file: http://bugs.python.org/file26990/issue-14649-2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Andrew Svetlov]

Andrew Svetlov added the comment:

Ok, I'm +0 for that.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Chris Jerdonek added the comment:

The problem seems to be restricted only to DocTestSuite.  testmod and 
DocTestFinder both seem to work fine.  DocTestSuite calls DocTestFinder but not 
the other way around: nothing calls DocTestSuite.

If in certain versions we do not treat raising the exception as a bug (e.g. in 
already-released versions), then I am hoping we can at least document the 
behavior as a known issue in those versions and still change the exception 
message to be more accurate and helpful.

Aside from ensuring that docstrings are present, another way to work around the 
issue is to pass a DocTestFinder to DocTestSuite with exclude_empty set to 
False, as in the following code:

finder = DocTestFinder(exclude_empty=False)
suite = DocTestSuite(module, test_finder=finder)

Depending on what we decide for which versions, I am willing to create two 
patches: one for the documentation and exception message change (e.g. for 
stable versions), and another to drop raising the exception (e.g. for future 
versions).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[R. David Murray]

R. David Murray added the comment:

That's a good question.  Perhaps you could argue for it as a bug fix since it 
doesn't seem to be documented either way...except for the 'exclude_empty' 
argument of testmod.  If testmod throws an error when there are no docstrings 
instead of returning a 0 test count or (if exclude_empty is true) skipping the 
module, then I think we can treat it as a bug and fix it in all active versions.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Chris Jerdonek added the comment:

> I suspect the intent was to raise an error if there were no *tests*, not if 
> there were no docstrings.

That, or the implementor thought that if no docstrings were found, then that 
might indicate something went wrong with the parsing.

For background purposes, here is the justification in the code comment:

elif not tests:
# Why do we want to do this? Because it reveals a bug that might
# otherwise be hidden.
raise ValueError(module, "has no tests")

> I have a couple of test runners in various projects that run doctest on *all* 
> modules

Same here, which is how I noticed the issue.

> So if anything I think this exception should be dropped.  If the report is 
> that no tests are run, that should be enough of an indication that something 
> is wrong

I would be in favor of this. The unittest module's test discovery, for example, 
does not raise an exception if it finds no tests (to my knowledge).  In what 
versions would we be able to make this change?

I could prepare another patch.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Chris Jerdonek added the comment:

> Every doctest is docstring from tested module, but not every docstring is a 
> valid doctest.

Actually, I'm not sure even this is correct.  doctest will form a unittest 
*TestSuite* from a module if it has docstrings, but it will not necessarily 
create a test for each docstring.  You can see this by running code like the 
following on a target module that contains an empty docstring:

suite = doctest.DocTestSuite('doctest_target')
print(repr(suite))
print(suite.countTestCases())

which outputs--


0

So while "tests" (in the doctest code) evaluates to True (because it is a 
TestSuite instance), it still "has no tests."  If it wants to check for tests, 
it should probably be evaluating tests.countTestCases(), as David suggested.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[R. David Murray]

R. David Murray added the comment:

Chris isn't suggesting changing the behavior, just the error message.  I agree 
with his change for exactly the reason you say: currently doctest doesn't 
complain if a module does have docstrings but none of those docstrings are 
tests.  So a module could have no tests without the error being raised.

However, I suspect that the current behavior is a bug.  I suspect the intent 
was to raise an error if there were no *tests*, not if there were no 
docstrings.  I don't think it is a bug that should be fixed, though.  I have a 
couple of test runners in various projects that run doctest on *all* modules, 
the goal being to pick up any doctests that there are or that I may add, even 
though not all modules have docstrings that contain doctests.  The doctest 
version of TestCase-based discovery.

So if anything I think this exception should be dropped.  If the report is that 
no tests are run, that should be enough of an indication that something is 
wrong, without raising an error, as it has been for the case where there are 
docstrings but no tests (we do not as far as I know have a bug or feature 
request that the current behavior is inadequate).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Andrew Svetlov]

Andrew Svetlov added the comment:

I think current behavior is correct.
Every doctest is docstring from tested module, but not every docstring is a 
valid doctest.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Changes by Chris Jerdonek :


--
nosy: +asvetlov

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Changes by Chris Jerdonek :


--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Chris Jerdonek  added the comment:

I have attached a patch with tests.

--
keywords: +easy, patch
stage:  -> patch review
Added file: http://bugs.python.org/file26269/issue-14649-1.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

Changes by Chris Jerdonek :


--
versions: +Python 3.3

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue14649] doctest.DocTestSuite error misleading when module has no docstrings

[Chris Jerdonek]

New submission from Chris Jerdonek :

When invoking doctest.DocTestSuite's constructor with a module that has no 
docstrings, doctest raises the following exception:

...
File 
"/opt/local/Library/Frameworks/Python.framework/Versions/3.2/lib/python3.2/doctest.py",
 line 2280, in DocTestSuite
raise ValueError(module, "has no tests")
ValueError: (, 'has no tests')

The error message is misleading because the exception is not raised for modules 
that have docstrings but no doctests, only for modules that have no docstrings.

To be accurate, the message should be something like 'has no docstrings'.

--
components: Library (Lib)
messages: 159022
nosy: cjerdonek
priority: normal
severity: normal
status: open
title: doctest.DocTestSuite error misleading when module has no docstrings
type: behavior
versions: Python 2.7, Python 3.2

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com