-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
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
Conversation
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 usernameWe couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames: 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! |
bac17b7
to
ea26720
Compare
@@ -83,7 +83,10 @@ Here's a complete but small example module:: | |||
|
|||
if __name__ == "__main__": | |||
import doctest | |||
doctest.testmod() | |||
import sys | |||
fail, _ = doctest.testmod() |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
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 @rhettinger, what's your opinion from a teaching perspective? |
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:
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? |
The change is supposed to fix the |
I understand, but I don’t know if you get my viewpoint 🙁 I proposed to not change the existing examples, it’s good that these are complete running examples. |
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. |
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