Skip to content

bpo-40372: Make doctest example programs exit with code 1 if any test fails #19673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

AndiDog
Copy link

@AndiDog AndiDog commented Apr 23, 2020

Build commands should fail on test error. That includes calling files' main block by executing them directly.

Therefore, the documented examples are changed to return an error code if any single test has failed (but still succeeds even if no tests are found, which is probably overkill here and not always desired).

https://bugs.python.org/issue40372

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@AndiDog

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Apr 23, 2020
@@ -83,7 +83,10 @@ Here's a complete but small example module::

if __name__ == "__main__":
import doctest
doctest.testmod()
import sys
fail, _ = doctest.testmod()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be easier to have a new argument in testmod(), e.g.:

doctest.testmod(exit_on_error=True)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Such a new feature would require a new Python version to be released. There's already one way to do it, even supported in Python 2.

@csabella
Copy link
Contributor

csabella commented May 23, 2020

I'm not sure if this would be a beneficial change to the docs. The current examples show the minimal that is needed to run doctest.testmod(). An example that includes the failures is at the end of the documentation and there is other information included in the description of testmod that a user can learn about when they want more info.

@rhettinger, what's your opinion from a teaching perspective?

@csabella csabella requested a review from rhettinger May 23, 2020 01:42
@merwok
Copy link
Member

merwok commented May 23, 2020

It’s not strictly incorrect to have doctest always return 0: after all, it ran fine itself, even if not all tests reported success. Things like error return codes or proper stdout/stderr usage become needed for tools used in shell pipelines or test automation (tox or CI).

I think there is a gradient in doctest usage:

  • write doctests, run them manually from an interactive interpreter
  • add if __name__ == "__main__" with doctest.testmod to make it easier to run the tests
  • using a proper test runner like unitttest integration or pytest

The change proposed here would add a step: keep using doctest.testmod instead of a proper runner, but make it play well with shell pipelines. I think it would be fine to add a small paragraph about this, rather than editing existing sample code as in the current diff, to avoid too many details at once. Does that make sense?

@AndiDog
Copy link
Author

AndiDog commented May 23, 2020

The change is supposed to fix the __main__ case (no test runner), and most examples use that case right now. Probably the documentation should remove all running examples and have a new section "How to run" with the different options? (larger effort than what I proposed, but probably helpful to all users)

@merwok
Copy link
Member

merwok commented May 23, 2020

I understand, but I don’t know if you get my viewpoint 🙁
There isn’t agreement that doctest.testmod in the __main__ case is broken.

I proposed to not change the existing examples, it’s good that these are complete running examples.

@rhettinger
Copy link
Contributor

I concur that this isn't a useful update to the docs which aim to show how the tools are commonly used. The pattern in the PR is not standard.

@rhettinger rhettinger closed this May 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review docs Documentation in the Doc dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants