Issue 11354 fixing docs for lfnf by seanjedi · Pull Request #11361 · pytest-dev/pytest

seanjedi · 2023-08-27T22:36:54Z

closes #11354

Improved documentation for last-failed-no-failures to make functionality more clear.

to better explain functionality

nicoddemus

Hi @seanjedi,

Thanks a lot for the contribution!

I left a few comments, please take a look.

nicoddemus · 2023-08-28T20:55:13Z

doc/en/reference/reference.rst

+                            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.


Some minor tweaks:

Suggested change

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.

Thanks for the feedback, update the code here: 75e815f

nicoddemus · 2023-08-28T20:55:18Z

doc/en/how-to/cache.rst

+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.


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.

update the docs here: 75e815f

nicoddemus · 2023-08-28T20:55:53Z

src/_pytest/cacheprovider.py

        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)"


Please update this with the above suggestion.

update the docs here: 75e815f

nicoddemus · 2023-08-28T21:58:27Z

doc/en/how-to/cache.rst


-    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)


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?

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.

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.

That sounds good to me. 👍

Looks great!

https://pytest--11361.org.readthedocs.build/en/11361/how-to/cache.html

nicoddemus

Thanks @seanjedi for the patience to getting this ready for merging!

nicoddemus · 2023-08-28T23:23:40Z

doc/en/reference/reference.rst

      --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


I just remembered this text is actually automatically generated, but no harm updating it manually (it will be overwritten later).

Ohh I didn't know that actually.
Good to know, is that why the help code in cacheprovider and in reference are the same?

src/_pytest/cacheprovider.py

doc/en/how-to/cache.rst

Co-authored-by: Bruno Oliveira <[email protected]>

nicoddemus · 2023-08-29T00:14:58Z

Thanks again @seanjedi!

seanjedi added 3 commits August 26, 2023 13:56

docs: changing some docs for last-failed-no-failures

b98d293

to better explain functionality

docs: improve --lfnf docs to make functionality more clear

66e7a1f

Merge branch 'pytest-dev:main' into issue_11354-Fixing-docs-for-lfnf

3be5d3e

seanjedi mentioned this pull request Aug 27, 2023

Better explain --last-failed-no-failures in docs #11354

Closed

nicoddemus requested changes Aug 28, 2023

View reviewed changes

seanjedi added 2 commits August 28, 2023 14:36

Merge branch 'pytest-dev:main' into issue_11354-Fixing-docs-for-lfnf

722b487

docs: updating the docs to make --lfnf more clear

75e815f

nicoddemus reviewed Aug 28, 2023

View reviewed changes

seanjedi added 2 commits August 28, 2023 15:15

docs: Add a bit more to the doc for --lfnf

e431b8e

docs: quick change

e121c50

nicoddemus approved these changes Aug 28, 2023

View reviewed changes

Update src/_pytest/cacheprovider.py

5ac0a7a

nicoddemus reviewed Aug 28, 2023

View reviewed changes

doc/en/how-to/cache.rst Outdated Show resolved Hide resolved

docs: Added the name of the option we are changing the docs for

02f84bc

Co-authored-by: Bruno Oliveira <[email protected]>

nicoddemus enabled auto-merge (squash) August 28, 2023 23:54

nicoddemus merged commit 76ba7db into pytest-dev:main Aug 29, 2023

nicoddemus added the backport 7.4.x label Aug 29, 2023

github-actions bot mentioned this pull request Aug 29, 2023

[7.4.x] Issue 11354 fixing docs for lfnf #11364

Merged

		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.

Uh oh!

Comments

Conversation

seanjedi commented Aug 27, 2023

Uh oh!

nicoddemus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicoddemus Aug 28, 2023 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicoddemus Aug 28, 2023 • 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!

seanjedi Aug 28, 2023 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicoddemus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nicoddemus commented Aug 29, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nicoddemus Aug 28, 2023 •

edited

Loading

nicoddemus Aug 28, 2023 •

edited

Loading

seanjedi Aug 28, 2023 •

edited

Loading