gh-116622: Enable `test_doctest` on platforms that don't support subprocesses by mhsmith · Pull Request #116758 · python/cpython · GitHub

When testing on Android I noticed that test_doctest and test_zipimport_support (which calls test_doctest) were skipping entire modules because of missing subprocess support, even though only one test method actually required subprocesses. This PR makes the skip more selective.

Issue: Add support for Android as a target platform #116622

@sobolevn: You've done some work in this area recently; are you able to review this PR?

There's one test failure, but I don't think it's related.

Thanks, I like the idea, no need to skip the whole module just because one test requires the subprocess module.

What I don't like is that the diff is huge. Maybe we can do something like

def doctest_requires_subproccess(func):
    if not support.has_subprocess_support:
        func.__doc__ = None  # skip the doctest by setting it to `None`
    return func

@doctest_requires_subprocess
def test_CLI():  # as before, no changes
    ...

I've tried this locally with if True:

» ./python.exe -m test test_doctest -m test_CLI
Using random seed: 4265888589
0:00:00 load avg: 2.23 Run 1 test sequentially
0:00:00 load avg: 2.23 [1/1] test_doctest
test_doctest ran no tests

== Tests result: NO TESTS RAN ==

1 test run no tests:
    test_doctest

Total duration: 59 ms
Total tests: run=0 (filtered)
Total test files: run=1/1 (filtered) run_no_tests=1
Result: NO TESTS RAN

Good idea; I've generalized it slightly so it can accept any skip condition.

I want to double check our idea, maybe with @serhiy-storchaka ?
I know that you are also interested in doctest module and its tests.

There is one minor problem -- a skipped test just disappears from the output. It is not counted as skipped.

Yes, but I don't think that there's an easy way to fix that. Since the whole module is counted as a single DocTestSuite, we can see how it behaves with other skips (with -OO for example):

» ./python.exe -OO -m test test_doctest            
Using random seed: 1564874428
0:00:00 load avg: 2.01 Run 1 test sequentially
0:00:00 load avg: 2.01 [1/1] test_doctest

== Tests result: SUCCESS ==

1 test OK.

Total duration: 246 ms
Total tests: run=10 skipped=1
Total test files: run=1/1
Result: SUCCESS

Only 1 is reported to be skipped, while in reality all doctests were skipped in this module.

So, not reporting a single doctest is in line with that.

It is far from ideal, but we can set a docstring with a skipped doctest instead of None.

… skipped

In the current state a skipped doctest still isn't counted as skipped by unittest – it just shows as a passed test, even in the verbose log. So I've made a small change to DocTestCase to improve that.

I think that you are fixing this in the wrong PR :)
Please, create a new issue and a new PR about doctest skip feature.

And you can keep this PR focused on just doctest + subprocess behavior.

LGTM.

Although I agree that the change in DocTestCase deserves a separate issue or at least a separate PR (you can also make it a part of the larger issue #108885). It is larger and more interesting than your initial PR. But anyway, both changes LGTM.

I also think that we can add @doctest_skip and @doctest_skip_if decorators to the stdlib. They might be useful for others. This should be a new issue as well :)

I agree that the change in DocTestCase deserves a separate issue or at least a separate PR

OK, I've split it out into #117297.

I also think that we can add @doctest_skip and @doctest_skip_if decorators to the stdlib. They might be useful for others.

I looked into allowing doctests to use the existing unittest.skip decorators, but that isn't so easy, because by the time we come to run the test, we no longer have a reference to the function whose docstring it came from.

Anyway, I've already got way deeper into this area than I was planning, so I'll leave that to someone else. 😄

than I was planning, so I'll leave that to someone else.

I will take it from here, thank you for this PR! 👍

Thanks! Just one comment.

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

@sobolevn: Are you able to merge this PR?

Yes, sure! Thanks a lot for working on this 👍

What do you think about 3.12 backport?

On 3.12 the only platforms that don't support subprocesses are Emscripten and WASI, so a backport would slightly improve test coverage for them.

However, to actually report the test as skipped would require #117297, which shouldn't be backported because it's a behavior change.

…t subprocesses (python#116758) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

Enable test_doctest on platforms that don't support subprocesses

4264ca9

bedevere-app Bot added the awaiting review label Mar 13, 2024

bedevere-app Bot mentioned this pull request Mar 13, 2024

Add support for Android as a target platform #116622

Closed

bedevere-app Bot added the tests Tests in the Lib/test dir label Mar 13, 2024

Fix whitespace

7d4f0fb

sobolevn reviewed Mar 21, 2024

View reviewed changes

mhsmith added the skip news label Mar 21, 2024

mhsmith added 2 commits March 21, 2024 18:37

Merge branch 'main' into doctest-subprocess

b735bd7

Switch to a doctest_skip_if decorator

ffee805

sobolevn approved these changes Mar 21, 2024

View reviewed changes

serhiy-storchaka self-requested a review March 21, 2024 21:19

serhiy-storchaka reviewed Mar 21, 2024

View reviewed changes

Make DocTestCase report as skipped if all examples in the doctest are…

d022538

… skipped

mhsmith removed the skip news label Mar 27, 2024

serhiy-storchaka approved these changes Mar 27, 2024

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting review labels Mar 27, 2024

sobolevn reviewed Mar 27, 2024

View reviewed changes

Comment thread Lib/test/test_doctest/sample_doctest_skip.py Outdated

mhsmith mentioned this pull request Mar 27, 2024

gh-117294: Report DocTestCase as skipped if all examples in the doctest are skipped #117297

Merged

Move most of previous commit into python#117297

405d679

mhsmith added the skip news label Mar 27, 2024

sobolevn reviewed Mar 29, 2024

View reviewed changes

Comment thread Lib/test/test_doctest/test_doctest.py Outdated

sobolevn mentioned this pull request Mar 29, 2024

Consider adding doctest.skip_if decorator #117364

Open

Don't modify docstrings when running in optimized mode

f9e9687

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

sobolevn approved these changes Mar 31, 2024

View reviewed changes

sobolevn merged commit 22b25d1 into python:main Apr 9, 2024

bedevere-app Bot removed the awaiting merge label Apr 9, 2024

sobolevn mentioned this pull request Apr 10, 2024

[3.12] gh-117692: Fix AttributeError in DocTestFinder on wrapped builtin_or_method (GH-117699) #117708

Merged

diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024

pythongh-116622 : Enable test_doctest on platforms that don't suppor…

c94ff86

…t subprocesses (python#116758) Co-authored-by: Nikita Sobolev <mail@sobolevn.me>

-                  Traceback (most recent call last):
-                    ...
-                  FileNotFoundError: [Errno ...] ...nosuchfile...
+              if support.has_subprocess_support:

gh-116622: Enable `test_doctest` on platforms that don't support subprocesses by mhsmith · Pull Request #116758 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-116622: Enable test_doctest on platforms that don't support subprocesses - #116758

gh-116622: Enable test_doctest on platforms that don't support subprocesses#116758sobolevn merged 7 commits intopython:mainpython/cpython:mainfrom mhsmith:doctest-subprocessmhsmith/cpython:doctest-subprocessCopy head branch name to clipboard

Conversation

mhsmith commented Mar 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mhsmith commented Mar 21, 2024

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

sobolevn Mar 21, 2024

Choose a reason for hiding this comment

Uh oh!

mhsmith Mar 21, 2024

Choose a reason for hiding this comment

Uh oh!

sobolevn commented Mar 21, 2024

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

sobolevn commented Mar 22, 2024

Uh oh!

serhiy-storchaka commented Mar 22, 2024

Uh oh!

mhsmith commented Mar 27, 2024

Uh oh!

sobolevn commented Mar 27, 2024

Uh oh!

serhiy-storchaka left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mhsmith commented Mar 27, 2024

Uh oh!

sobolevn commented Mar 27, 2024

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mhsmith commented Apr 4, 2024

Uh oh!

sobolevn commented Apr 9, 2024

Uh oh!

sobolevn commented Apr 9, 2024

Uh oh!

mhsmith commented Apr 9, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gh-116622: Enable test_doctest on platforms that don't support subprocesses#116758
sobolevn merged 7 commits into
python:mainfrom
mhsmith:doctest-subprocess

mhsmith commented Mar 13, 2024 •
edited
Loading

serhiy-storchaka left a comment •
edited
Loading