-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Issue 11354 fixing docs for lfnf #11361
Issue 11354 fixing docs for lfnf #11361
Conversation
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.
doc/en/reference/reference.rst
Outdated
Determines whether to execute tests when there | ||
are no previously (known) failures or when no | ||
cached ``lastfailed`` data was found. | ||
This option governs the behavior of ``--if``. | ||
Default ``all`` runs all tests with no known failures. | ||
``none`` avoids tests and exits if no failures. |
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.
Some minor tweaks:
Determines whether to execute tests when there | |
are no previously (known) failures or when no | |
cached ``lastfailed`` data was found. | |
This option governs the behavior of ``--if``. | |
Default ``all`` runs all tests with no known failures. | |
``none`` avoids tests and exits if no failures. | |
With ``--lf``, determines whether to execute tests when there | |
are no previously (known) failures or when no | |
cached ``lastfailed`` data was found. | |
``all`` (the default) runs the full test suite again. | |
``none`` just emits a message about no known failures and exits successfully. |
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.
Thanks for the feedback, update the code here: 75e815f
doc/en/how-to/cache.rst
Outdated
The ``--last-failed-no-failures`` option governs the behavior of ``--if``. | ||
Default ``all`` runs all tests with no known failures. ``none`` avoids tests and exits successfully when there are no known failures. |
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.
IMHO this does not add much to what we already have above (and people found lacking in the first place).
Feel free to not only add to this section, but rewrite it entirely if you think it would be better.
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.
update the docs here: 75e815f
src/_pytest/cacheprovider.py
Outdated
@@ -499,7 +499,11 @@ def pytest_addoption(parser: Parser) -> None: | |||
dest="last_failed_no_failures", | |||
choices=("all", "none"), | |||
default="all", | |||
help="Which tests to run with no previously (known) failures", | |||
help="Determines whether to execute tests when there are no previously (known)" |
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.
Please update this with the above suggestion.
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.
update the docs here: 75e815f
doc/en/how-to/cache.rst
Outdated
|
||
.. code-block:: bash | ||
|
||
pytest --last-failed --last-failed-no-failures all # run all tests (default behavior) | ||
pytest --last-failed --last-failed-no-failures none # run no tests and exit | ||
pytest --last-failed --last-failed-no-failures all # run the full test suite again (default behavior) |
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.
I don't think we need to use a command line example (I know that's how it was written before, of course), perhaps it would be better to just list the options, something like:
* ``all``: when there are no known test failures, runs all tests (the full test suite). This is the default.
* ``none``: when there are no known test failures, just emits a message stating this and exit successfully.
What do you think?
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.
I don't think it would be more confusing to include the command line example, and it hones in the idea of how to use the command and that it is utilized with the option --last-failed
I think that the current documentation is a bit more concise in that the above docstring details what the command does and that the two command line examples show the two options with the comment giving enough information that a user should understand what they both do.
However, that is my opinion, if you think that is wrong feel free to let me know.
However, I do think that I can add a bit more to the current version of it, which I will do soon.
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.
You got a point, but that leads to hard-to-read horizontal bar:
https://pytest--11361.org.readthedocs.build/en/11361/how-to/cache.html
That's why I think perhaps listing the options explicitly would be better... perhaps we can have both? In the command-line example, use a shorter explanation (the previous one is good I think), then we can use the long form in the listing.
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.
That sounds good to me. 👍
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.
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.
Thanks @seanjedi for the patience to getting this ready for merging!
@@ -1892,8 +1892,11 @@ All the command-line flags can be obtained by running ``pytest --help``:: | |||
tests. Optional argument: glob (default: '*'). | |||
--cache-clear Remove all cache contents at start of test run | |||
--lfnf={all,none}, --last-failed-no-failures={all,none} | |||
Which tests to run with no previously (known) | |||
failures | |||
With ``--lf``, determines whether to execute tests when there |
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.
I just remembered this text is actually automatically generated, but no harm updating it manually (it will be overwritten later).
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.
Ohh I didn't know that actually.
Good to know, is that why the help code in cacheprovider and in reference are the same?
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.
Yes!
Co-authored-by: Bruno Oliveira <bruno@soliv.dev>
Thanks again @seanjedi! |
closes #11354