sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.61k stars 2.13k forks source link

Doctest SKIP directive ignored by testcode:: code block #13103

Open cacti77 opened 2 weeks ago

cacti77 commented 2 weeks ago

Describe the bug

doctest is ignoring the +SKIP directive in my rst testcode:: samples.

How to Reproduce

The docs at https://www.sphinx-doc.org/en/master/usage/extensions/doctest.html#directive-testcode imply that I can append doctest directives to the end of a line in a code sample. However they don't appear to be honoured.

Simple example from an rst file:

.. testcode::

   x = 10
   x += 10  # doctest: +SKIP
   print(x)

.. testoutput::

   10

But make doctest fails with:

Failed example:
    x = 10
    x += 10  # doctest: +SKIP
    print(x)
Expected:
    10
Got:
    20

It's clearly running the line it should be skipping. At first I thought the default trim-doctest-flags flag was effectively stripping out the doctest directive before obeying it. But setting the :no-trim-doctest-flags: option for testcode:: doesn't change the result.

I've only tried the SKIP directive, but it's possible this issue applies to other doctest directives too.

Environment Information

Windows 10
Python 3.9.20

>sphinx-build --version
sphinx-build 7.3.7

Sphinx extensions

My conf.py has these extensions:

extensions = [
    'sphinx.ext.autodoc',
    'sphinx.ext.viewcode',
    'sphinx.ext.mathjax',
    'sphinx.ext.todo',
    'sphinx.ext.napoleon',
    'sphinx.ext.intersphinx',
    'sphinx.ext.doctest'
]

Additional context

No response

jayaddison commented 2 weeks ago

Thanks for the report @cacti77 - I could be mistaken, but I think there are two things to note here:

.. testcode::
   :options: +SKIP

   x = 10
   x += 10
   print(x)

.. testoutput::

   10

This should skip the test, meaning that it won't contribute towards failing the build despite the possibility (actuality, in this case) that the in it fails to evaluate to the expected output.

I think we should update the Sphinx documentation to clarify the latter point.

electric-coder commented 1 week ago

The doctest.SKIP flag seems designed to skip entire tests, not individual lines.

That was also my impression, although the Python docs state it clearly the way it's phrased still leaves margin for doubt.

cacti77 commented 1 week ago

@jayaddison : ok, you're right, I misunderstood. I thought # doctest: +SKIP at the end of a line skipped only that line rather than the entire example.

It would be really useful to have a feature that allows the skipping of specific lines within an example, because then I could show code in examples but skip running certain lines that might cause a problem in doctest; like popping up matplotlib plots that can't be closed without user intervention, for example. However you'd also need a way of suppressing those flags from being shown in other builders (like html).

The bit that misled me in the docs was where it says:

doctest flags (comments looking like # doctest: FLAG, ...) at the ends of lines

for the doctest, testcode and testoutput directives. You can close this issue if you want, or leave it open to clarify the docs; up to you.

jayaddison commented 1 week ago

Thanks @cacti77. It seems I spoke too soon; after some more experimentation with doctest -- using it independently of Sphinx -- it can indeed skip individual lines of test code. Perhaps we could consider this a bug, and fix the implementation instead of changing the documentation. I'll need to figure out a bit more about why the difference occurs.

The additional context from your comment is useful too; if I understand it, the goal is to produce documentation that contains example code users can read and run for themselves, and also to run that code during project builds, to ensure that it continues to produce the expected results/output.

Edit: insert some missing/clarifying words

jayaddison commented 1 week ago

(...and for the sample code that is presented in the documentation to omit any irrelevant doctest-configuration comments)

jayaddison commented 1 week ago

Ok, yep - I think we have a bug here somewhere. It could perhaps relate to the difference between the doctest (interleaved test/output doctesting) and testcode+testoutput (doctesting using paired directives with code and expectations).

I'm able to get the following to pass:

.. doctest::

   >>> x = 10
   >>> x += 10  # doctest: +SKIP
   >>> print(x)
   10

But this fails (as reported):

.. testcode::

   x = 10
   x += 10  # doctest: +SKIP
   print(x)

.. testoutput::

   10
jayaddison commented 1 week ago

@cacti77 A short summary / tl;dr here is that if you can refactor to use the doctest directive instead of testcode+testoutput, you may be able to achieve what you want (unless there is some other constraint that makes the latter preferable).

Notes

There are some implementation details of the doctest directive compared to the testcode/testoutput-pair directives that can cause quirky behaviour.

In particular: the behaviour here stems from the fact that doctest example code is evaluated one-line-at-a-time -- meaning that when doctest: +SKIP is found on a single line of code, only that line of code is skipped, but other lines/statements are evaluated.

When a testcode example is evaluated by doctest, the entire (potentially multiline) code is evaluated within a single operation. What I don't yet understand is why the presence of the doctest-skip comment in this case doesn't skip evaluation of the entire example. I think this may relate to either/both of:

...or potentially something about the difference in the way that the test object is constructed in each of the two separate paths. I'm not sure yet.

cacti77 commented 1 week ago

@jayaddison - thanks for all the comments.

I'm glad doctest independently of Sphinx can skip individual lines of code. That's what I would expect if it is permissible to add # doctest: +SKIP to the end of line. Perhaps doctest's own docs need clarifying on this point.

Given that behaviour in doctest then I don't understand why you say this:

What I don't yet understand is why the presence of the doctest-skip comment in this case doesn't skip evaluation of the entire example.

Surely the intention should be to apply the same behaviour as in doctest - i.e., only ignore specific lines appended with # doctest: +SKIP? Can't any such lines simply be deleted from the testcode code block before evaluating any remaining lines?

if you can refactor to use the doctest directive instead of testcode+testoutput, you may be able to achieve what you want (unless there is some other constraint that makes the latter preferable).

Thanks! But for the rst tutorial docs I think the testcode blocks look much cleaner than the doctest blocks in the generated html because the code samples aren't cluttered with interpreter prompts. Re the context, yes, doctest is fantastic for testing code samples and so it's frustrating to not be able to test 9 out of 10 lines just because one of them needs the user to close a plot, or Apache Spark is needed to execute that line for example. Right now I'm just commenting out those annoying lines even though a user would need to run them in practice; not ideal:).

electric-coder commented 1 week ago

Perhaps doctest's own docs need clarifying on this point.

If you can spare the time this would be worth an FR at the CPython repository, I find the: ",do not run the example at all." clause very misleading and prone to confusion.

cacti77 commented 1 week ago

@electric-coder (Err, what's an FR?!) This sentence in doctest.SKIP threw me too:

When specified, do not run the example at all.

However, the doctest docs as a whole suggest an 'example' corresponds to a single interactive prompt. E.g., in this code snippet:

    >>> [factorial(n) for n in range(6)]
    [1, 1, 2, 6, 24, 120]
    >>> factorial(30)
    265252859812191058636308480000000
    >>> factorial(-1)
    Traceback (most recent call last):
        ...
    ValueError: n must be >= 0

there are 3 prompts and hence 3 'examples', each of which could be appended by their own doctest directive (such as +SKIP).

If this interpretation is correct then it makes sense to support doctest directives on individual lines in doctest blocks, because that would be consistent with doctest itself.

It comes back to this comment by @jayaddison. Does sphinx.ext.doctest support doctest directives at the per line level for doctest-style code blocks, but only apply them at the level of the whole code block for code-output-style blocks? This is what happens now, but maybe an exception should also be thrown if any doctest directives are detected on lines within testcode:: or testoutput:: blocks. This would be a breaking change though, and the trim-doctest-flags and no-trim-doctest-flags options would then also be redundant for code-output-style blocks.

Or take the easy route out: leave it as it is, but maybe change the docs for testcode:: or testoutput:: blocks to make it clear that any doctest directives at the ends of lines in the code sample will be ignored.