classification
Title: make doctest in CPython has failures
Type: behavior Stage: resolved
Components: Documentation, Tests Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: 27202 27203 27204 27205 27206 27207 27208 27209 27210 28860 34962 Superseder:
Assigned To: Mariatta Nosy List: Jelle Zijlstra, Mariatta, berker.peksag, brett.cannon, ezio.melotti, marco.buttu, mdk, python-dev, r.david.murray, rhettinger, terry.reedy, xtreak, zach.ware
Priority: low Keywords: patch

Created on 2016-06-03 21:13 by Jelle Zijlstra, last changed 2018-10-30 20:00 by mdk. This issue is now closed.

Files
File name Uploaded Description Edit
datetime.rst.patch marco.buttu, 2016-12-04 10:53 review
make_doctest_ok.patch marco.buttu, 2016-12-07 18:45 review
issue27200_3rd.patch marco.buttu, 2016-12-09 16:23 review
configparser_ctypes_copyreg.patch marco.buttu, 2017-01-30 13:19 review
Pull Requests
URL Status Linked Edit
PR 240 merged marco.buttu, 2017-02-22 20:54
PR 401 merged marco.buttu, 2017-03-02 19:41
PR 604 merged marco.buttu, 2017-03-10 19:48
PR 616 merged marco.buttu, 2017-03-11 15:10
PR 821 closed marco.buttu, 2017-03-26 08:18
Messages (34)
msg267166 - (view) Author: Jelle Zijlstra (Jelle Zijlstra) * (Python triager) Date: 2016-06-03 21:13
I'm going to gather together a patch to fix some of these issues.
msg267209 - (view) Author: Jelle Zijlstra (Jelle Zijlstra) * (Python triager) Date: 2016-06-04 00:48
Added a number of more specific issues with patches. I'll add more patches after the current ones are resolved.
msg267275 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-06-04 17:03
Thanks for all of these patches, but I'm -1 on committing them. We already have an extensive test suite and I'm pretty sure that all of these examples are already tested in Lib/test [*]. Learning reST is already hard for contributors and we shouldn't make it harder by introducing new directives like 'testsetup'.

* We have also converted some doctests to proper unittests in Lib/test in the past.
msg267278 - (view) Author: Jelle Zijlstra (Jelle Zijlstra) * (Python triager) Date: 2016-06-04 17:11
I would consider this a test of the quality of the documentation, not of the implementation. It's important that the examples in the documentation are actually correct. For example, a doctest in Doc/library/datetime.rst passes an "hours=" keyword argument to datetime.time when "hour=" is correct. It's going to be hard to find such bugs in examples without an automated check like `make doctest`.

I'm open to simplifying the patches by removing testsetup blocks. For what it's worth though, the docs already contain 17 testsetup blocks.
msg267284 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-06-04 17:25
Yes, the point here is to test the docs, not the code, and is definitely worthwhile.  If we can get make doctest to pass, then the docs buildbot can start running it.
msg267373 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-06-05 04:21
I have also found some markup errors via 'make doctest' so I agree that this can be helpful :)
msg272315 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-08-10 06:06
New changeset f3a04d19e5cb by Zachary Ware in branch '3.5':
Issue #27200: Fix doctests in Doc/library/hashlib.rst
https://hg.python.org/cpython/rev/f3a04d19e5cb

New changeset d0302d8ecbc1 by Zachary Ware in branch 'default':
Issue #27200: Merge with 3.5
https://hg.python.org/cpython/rev/d0302d8ecbc1
msg272316 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2016-08-10 06:19
All of the current dependencies have been committed.  I still see failures in:

Doc/faq/programming.rst
Doc/library/configparser.rst (also leaves Doc/example.ini)
Doc/library/copyreg.rst
Doc/library/ctypes.rst
Doc/library/datetime.rst
Doc/library/functions.rst
Doc/library/ipaddress.rst
Doc/library/pathlib.rst
Doc/library/re.rst
Doc/library/reprlib.rst
Doc/library/shlex.rst
Doc/library/turtle.rst (possibly only due to lack of usable tkinter)
Doc/library/unittest.mock-examples.rst
Doc/library/unittest.mock.rst
Doc/library/urllib.parse.rst
Doc/library/weakref.rst

For a total of 450 failures (1 in setup code) out of 1726 tests on 3.6.  Also, some test leaves 'Doc/foo'.
msg272317 - (view) Author: Jelle Zijlstra (Jelle Zijlstra) * (Python triager) Date: 2016-08-10 06:31
Thanks for committing all of these patches! I have local patches for some more tests; I'll try to find some time to upload them.
msg272355 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2016-08-10 18:06
Thanks for creating all the patches!  If you create further issues for this, feel free to make me nosy.
msg282309 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-12-03 23:15
#28860 has a patch for configparser.  Jelle, if you have more patches for some of the items listed above, please upload them so Marco can focus on the others.
msg282310 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-12-03 23:19
#10225, still open, has a old 3.2 patch that might be useful.  If it is not, it should be closed.  Note that Raymond does not like the patch for the sorting help doc.
msg282330 - (view) Author: Marco Buttu (marco.buttu) * Date: 2016-12-04 10:53
Here is a patch that makes all Doc/library/datetime.rst doctests pass. The tests only pass with Python 3.6 and 3.7 (the optional ``timespec`` argument of ``datetime.isoformat()`` has been introduced in Python 3.6).

To apply the patch you should agree with issue 28863. In particular, to make the patch effective, as explained in issue 28863, you should:

* apply the Doc/conf.py patch of issue 28863.
* rename Doc/includes/tzinfo-examples.py to Doc/includes/tzinfo_examples.py

About the Raymond disappointment expressed in #10225, I see two ways to make both Raymond happy and the code examples tested. For the examples included from Doc/includes, using the ``literalinclude`` Sphinx directive, we can just add the Doc/includes path in Doc/conf.py (see issue 28863). At this point we can import them in the shell code examples (or in the ``setupcode``). In the datetime.rst.patch in attachment, I make use of this solution. Another option, for short snippets of code, not included from Doc/includes, is to use the ``testcode`` Sphinx directive.
msg282651 - (view) Author: Marco Buttu (marco.buttu) * Date: 2016-12-07 18:45
This last patch (make_doctest_ok.patch) makes all doctests pass (Sphinx 1.5, Python 3.6). Before applaying the patch there are 466 failures in 2096 tests, and after we have more tests (2414) and 0 failures. Actually, sometimes 3 tests fail in Doc/library/unittest.mock-examples.rst, but I did not get why :/
msg282788 - (view) Author: Marco Buttu (marco.buttu) * Date: 2016-12-09 16:23
I isolated all snippets in the unittest.mock documentation, and now all doctests pass without surprises :-)
msg284499 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2017-01-02 20:42
I left a few comments on rietveld,
In general, if making the doctest pass entails adding superfluous code (e.g. sorted(), <BLANKLINES>, #doctest: +SKIP (if visible), setups/teardowns, or even whole files) or removing details that might be useful (ids or filenames in reprs), I'd rather skip the snippet altogether.

Keep in mind that most of those snippets are there to document, not for testing, so imho the priority should be keeping them clear and to the point.
The behavior of those snippets should already be covered by unittest, so the only downside of skipping is that, if the behavior changes, the examples will be wrong until someone notices.

If others agree, this should make the patch simpler and easier to review; if not, breaking it down in 3-4 patches that address separate issue might also help reviewers.
msg284520 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-01-03 01:00
I agree with Ezio.
msg284548 - (view) Author: Marco Buttu (marco.buttu) * Date: 2017-01-03 11:01
Hi Ezio, thanks for your time :) The +SKIP option and the testsetup and teardown directives are not visible in the output. The only extra code in the output is sorted(). I agree with you about the use of sorted(), and I think we do not have to add extra code just to make the example passes. But as you can see in issue28860, I was asked to add it. Before going on, I think we should have a clear policy about it.

I also think the examples are there do document, but I believe there are good reasons to test them. In fact, unittests and doctests have complete diffent purposes: the goal of doctests is to verify the examples work properly, that's all. An example could be wrong also if all the related unittests pass and it is still updated: there could be a typo in the code or in the output, a missing import, and other kind of errors or lacks that make the example not working, getting confused the reader. Unluckily there is not just the downside you pointed out :/

Ok to break the patch in more patches, but I did not get if you want me to open a new issue for each patch, or attach all the patches here in this issue. Thanks again
msg284560 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2017-01-03 13:22
> The +SKIP option and the testsetup and teardown directives are not visible in the output.

So feel free to use them when needed :)

> But as you can see in issue28860, I was asked to add it. Before going
> on, I think we should have a clear policy about it.

If the priority is making the test pass, then adding sorted() is a viable solution -- if keeping the examples simple and to the point has higher priority, then the test could be skipped.  The priority depends on the case.

> An example could be wrong also if all the related unittests pass and
> it is still updated: there could be a typo in the code or in the 
> output, a missing import, and other kind of errors or lacks that make > the example not working, getting confused the reader.

True.  In general, I think most of the current examples are fine, and whoever adds new ones or changes the behavior should take care that the examples (still) work.  If there is a mistake in an untested example, hopefully someone will report it.  While it's true that these mistakes might cause confusion, the same could be said for other changes done to the code to make the test pass (Why do I need to call sorted()/list()/etc.?  Why there are '...' in the output, is this an actual terminal session? etc.).     

> Ok to break the patch in more patches, but I did not get if you want
> me to open a new issue for each patch, or attach all the patches here 
> in this issue. Thanks again

It's probably fine to keep the discussion and patches on this issue, unless you prefer to handle discussions about specific parts separately -- just make sure that it's clear from the file description what patches should be applied.
As for the number of patches, probably 2-4 files are enough (e.g. one file could include all the trivial addition of +SKIP/.. doctest::, another the more "controversial" changes that add new files).
msg284628 - (view) Author: Marco Buttu (marco.buttu) * Date: 2017-01-04 11:06
Thinking a little bit about it, I believe that the easiest way to proceed is to make one patch for every file or two, with the only purpose to make their doctests to pass. Otherwise if I make a patch with all the doctest directives, or just with the +SKIP options, we will have hundreds of failures, and it will be really hard to understand what is going wrong, also because the tests are not isolated, and we will have some random failures. Reducing the number of failures by making a patch with all the doctest/testsetup/testcleanup directives is not a solution, because we eventually fall again into a big patch, and the doctests are still not pass too. 

That is why I think having one patch for two files will reduce a lot the effort in making and reviewing the patch. We will end up in about 9 patches, and it will be really easy to review most of them (I think couple of minutes). It will be also easy to test them. For instance, if the patch involves the files Doc/library/copyreg.rst and Doc/library/ctypes.rst, we just have to to to the Doc/ directory and run: 

  $ sphinx-build -b doctest . build/doctest \
  > library/copyreg.rst \
  > library/ctypes.rst

expecting 0 failures. 

Maybe having one new issue for each patch will also simplify the process, because the purpose of the patch will be well explained by the issue description and title. Let me know what you think, and thank you very much for your time.
msg286413 - (view) Author: Marco Buttu (marco.buttu) * Date: 2017-01-28 16:06
In order for the patches to work for every Python version (see #msg284622 ), I added a new feature to the Sphinx doctest extension, called pyversion. Starting from Sphinx 1.6, it will be possible to specify the Python version in order for the example to be tested. For instance, if I run the doctests with Python 3.3, then this test will be skipped:

.. doctest::
    :pyversion: > 3.4

    # Code example

It will be tested only when Sphinx is running on Python > 3.4. My question is: what about using this feature in the patches? I am asking this because Sphinx 1.6 will be released in about 7-10 months, so until then, if we want to use the :pyversion: feature we have to build the documentation using the master version of Sphinx. Please, let me know
msg286463 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2017-01-30 00:08
I'm -1 on building the docs from Sphinx master as I don't want to complicate our development process to deal with potential broken tooling. Basically if the doctests don't pass on a compiled checkout for the  version of Python the docs are being built for then the doctest is broken (i.e. no need to care about compatibility with older Python versions for docs meant for master).
msg286490 - (view) Author: Marco Buttu (marco.buttu) * Date: 2017-01-30 13:19
Thanks Brett :-) Following the suggestion of Ezio to split the patch in more patches (msg284560), and according to msg284628, here is a first patch for just three files. To run their doctests:

$ sphinx-build -b doctest . build/doctest \
library/configparser.rst \
library/ctypes.rst \
library/copyreg.rst
msg286510 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-01-31 03:36
Mariatta, would you please add your review comments to the proposed patches.  For the most part, we want the patches to make the minimal changes necessary to pass the doctests.  Also, clarity of the examples is more important than making the examples doctestable (i.e. perhaps added sorted() to the output makes it more predictable but it might also obfuscate the point being made).

In general, we would like to have the convenience of doc testing but should be unwilling to commit any atrocities along the way.  Ideally, the examples should be written in a way that doesn't garbage them up with <blankline> and whatnot (that is generally distracting to the reader and only serves doctest).  Similarly, make sure that clarity and cut-and-pastability don't get lost by adding ... PS2 prompts that uglify the examples.  In all cases, the intent of the example writer and the clarity of the example takes precedence over the doctests.

You should try out the patches and make sure the tests run, build the docs and make sure the examples are clean looking from a user point of view. If you find any issues or have questions, use rietveld for discussion (that is the "review" link to the right of the patch).
msg286511 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-01-31 03:37
Also, please make this 3.7 only.  There is no point in going back in time unless an example is found that is actually broken.
msg286514 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2017-01-31 03:45
Raymond Hettinger wrote:
> Similarly, make sure that clarity and cut-and-pastability don't get lost by adding ... PS2 prompts that uglify the examples.

Note that Sphinx adds a ">>>" button to the upper right of the example
that, when clicked, makes all of the prompts and output disappear so
that the code (and only the code) can be copied easily.  Not having
the ps2 prompts breaks that button.
msg286615 - (view) Author: Marco Buttu (marco.buttu) * Date: 2017-02-01 11:26
Thank you Raymond for your time. I just want to note that <blankline> does not distrac the reader, because it is not show in the output. Furthermore, to doctest an example we do not need to add the prompt (>>>): we can use the testcode/testoutput Sphinx directives.
msg290167 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-03-24 22:12
New changeset 909a6f626ff343937cd3f06fda996870e7890724 by Berker Peksag (Marco Buttu) in branch 'master':
bpo-27200: Fix doctests in programming.rst and datetime.rst (#401)
https://github.com/python/cpython/commit/909a6f626ff343937cd3f06fda996870e7890724
msg290347 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-03-24 23:15
New changeset b2a7c2f9862b28cf5db11ef837646ef0c589955c by Berker Peksag (Marco Buttu) in branch 'master':
bpo-27200: fix configparser, copyreg and ctypes doctests (#240)
https://github.com/python/cpython/commit/b2a7c2f9862b28cf5db11ef837646ef0c589955c
msg291617 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-04-13 14:18
New changeset 7b2491a6aa5cdc1f8f9e3fd9df91f29ee69aa982 by Berker Peksag (Marco Buttu) in branch 'master':
bpo-27200: Fix pathlib, ssl, turtle and weakref doctests (GH-616)
https://github.com/python/cpython/commit/7b2491a6aa5cdc1f8f9e3fd9df91f29ee69aa982
msg292431 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-04-27 12:23
New changeset e65fcde85abf6617508f2d6b77020e24b8ca6f6b by Berker Peksag (Marco Buttu) in branch 'master':
bpo-27200: Fix several doctests (GH-604)
https://github.com/python/cpython/commit/e65fcde85abf6617508f2d6b77020e24b8ca6f6b
msg328887 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2018-10-30 08:43
Sorry to bump this but to add some more context doctest related issues were fixed in issue34962 and is now enforced in CI.
msg328928 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2018-10-30 15:11
At the moment, the dependent issues are closed and there is just one open PR on this issue.

Julian, I am nosying you here because you merged #34962, which is properly a dependency of this.
msg328943 - (view) Author: Julien Palard (mdk) * (Python committer) Date: 2018-10-30 20:00
Closing PR and issue as already fixed and now enforced by CI.

Thanks again Marco for those already-merged fixes, it helped a lot ending the work at the last PyCon Fr sprints.
History
Date User Action Args
2018-10-30 20:00:05mdksetstatus: open -> closed
resolution: fixed
messages: + msg328943

stage: patch review -> resolved
2018-10-30 15:11:44terry.reedysetnosy: + mdk
dependencies: + make doctest does not pass :/
messages: + msg328928
2018-10-30 08:43:36xtreaksetnosy: + xtreak
messages: + msg328887
2017-04-27 12:23:36berker.peksagsetmessages: + msg292431
2017-04-13 14:18:01berker.peksagsetmessages: + msg291617
2017-03-26 08:18:43marco.buttusetpull_requests: + pull_request727
2017-03-24 23:15:15berker.peksagsetmessages: + msg290347
2017-03-24 22:12:02berker.peksagsetmessages: + msg290167
2017-03-11 15:10:56marco.buttusetpull_requests: + pull_request509
2017-03-10 19:48:51marco.buttusetpull_requests: + pull_request499
2017-03-02 19:41:50marco.buttusetpull_requests: + pull_request332
2017-02-22 20:54:43marco.buttusetpull_requests: + pull_request202
2017-02-01 11:26:26marco.buttusetmessages: + msg286615
2017-01-31 03:45:05zach.waresetmessages: + msg286514
2017-01-31 03:37:10rhettingersetpriority: normal -> low

messages: + msg286511
versions: - Python 2.7, Python 3.5, Python 3.6
2017-01-31 03:36:13rhettingersetassignee: Mariatta

messages: + msg286510
nosy: + rhettinger, Mariatta
2017-01-30 13:19:17marco.buttusetfiles: + configparser_ctypes_copyreg.patch

messages: + msg286490
2017-01-30 00:08:24brett.cannonsetmessages: + msg286463
2017-01-28 16:06:53marco.buttusetmessages: + msg286413
2017-01-04 11:06:08marco.buttusetmessages: + msg284628
2017-01-03 13:22:46ezio.melottisetmessages: + msg284560
2017-01-03 11:01:44marco.buttusetmessages: + msg284548
2017-01-03 01:00:13berker.peksagsetmessages: + msg284520
2017-01-02 20:42:40ezio.melottisetnosy: + ezio.melotti
messages: + msg284499
2016-12-09 16:42:29zach.waresetassignee: Jelle Zijlstra -> (no value)
stage: patch review
versions: + Python 3.7
2016-12-09 16:23:49marco.buttusetfiles: + issue27200_3rd.patch

messages: + msg282788
2016-12-07 18:45:59marco.buttusetfiles: + make_doctest_ok.patch

messages: + msg282651
2016-12-04 10:53:43marco.buttusetfiles: + datetime.rst.patch

nosy: + marco.buttu
messages: + msg282330

keywords: + patch
2016-12-03 23:19:33terry.reedysetmessages: + msg282310
2016-12-03 23:15:24terry.reedysetnosy: + terry.reedy
dependencies: + Fixed all the doctest failures in Doc/library/configparser.rst
messages: + msg282309
2016-08-10 18:06:41zach.waresetmessages: + msg272355
2016-08-10 06:31:37Jelle Zijlstrasetmessages: + msg272317
2016-08-10 06:19:31zach.waresettype: behavior
messages: + msg272316
components: + Documentation, Tests
versions: + Python 2.7, Python 3.5, Python 3.6
2016-08-10 06:06:08python-devsetnosy: + python-dev
messages: + msg272315
2016-08-09 21:53:35zach.waresetnosy: + zach.ware
2016-06-05 04:21:16berker.peksagsetmessages: + msg267373
2016-06-05 01:08:21Jelle Zijlstrasetassignee: Jelle Zijlstra
2016-06-04 17:25:21r.david.murraysetnosy: + r.david.murray
messages: + msg267284
2016-06-04 17:11:51Jelle Zijlstrasetmessages: + msg267278
2016-06-04 17:03:00berker.peksagsetnosy: + berker.peksag
messages: + msg267275
2016-06-04 00:48:01Jelle Zijlstrasetdependencies: + make doctest fails on 2.7 release notes, Failing doctests in Doc/faq/programming.rst, Failing doctests in Doc/howto/, Failing doctests in Library/collections.rst, Failing doctests in Doc/tutorial/, Failing doctests in Doc/whatsnew/3.2.rst, Failing doctests in Library/traceback.rst, Failing doctests in Library/email.*.rst, Failing doctests due to environmental dependencies in Lib/{nntp,ftp,diff}lib.rst
messages: + msg267209
2016-06-03 21:13:27Jelle Zijlstracreate