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: (<module '[snip]' from '[snip]'>, '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'.

I have attached a patch with tests.

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

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).

> 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--

<unittest.suite.TestSuite tests=[]>
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.

> 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.

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.

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).

Ok, I'm +0 for that.

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?

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.

> 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.

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

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.

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.

> 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.

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.

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.

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.

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?

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

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.

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.

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

Thanks, Chris.

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

Thanks a lot, David.