Skip to content
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

--lfnf does nothing unless you also pass --lf #11346

Closed
DetachHead opened this issue Aug 24, 2023 · 9 comments
Closed

--lfnf does nothing unless you also pass --lf #11346

DetachHead opened this issue Aug 24, 2023 · 9 comments
Labels
type: docs documentation improvement, missing or needing clarification

Comments

@DetachHead
Copy link
Contributor

this will run every test regardless of if there were any previous failures

pytest --lfnf=none

i have to pass both arguments for it to work:

pytest --lf --lfnf=none

i'm not sure if this is intended functionality for whatever reason, but if it is, it should be documented in the --help description. the current documentation seems a bit unclear:

  --lfnf={all,none}, --last-failed-no-failures={all,none}
                        Which tests to run with no previously (known) failures
@seanjedi
Copy link
Contributor

I would agree that the documentation is a bit unclear, I would also suggest updating the documentation to make t functionality of --lfnf more clear.

@nicoddemus
Copy link
Member

nicoddemus commented Aug 25, 2023

A PR to update the docs would be great.

Why not make ---lfnf imply --lf?

My understanding is that people often put the --lfnf configuration into their configuration file, so making that change would make all runs --lf.

@nicoddemus nicoddemus added the type: docs documentation improvement, missing or needing clarification label Aug 25, 2023
@seanjedi
Copy link
Contributor

seanjedi commented Aug 25, 2023

@nicoddemus I can work on the docs to make what this is supposed to do more clear, but I have an additional question, in which does --if and --ifnf=all do the same exact thing then?
It seems like the functionalities are a bit related, in which --if and --ifnf=all do the same thing, but --ifnf=none should do the inverse, regardless if --if is included.
However, it seems to me from the code that regardless what value --ifnf has, if the configuration has --if then it should rerun all previously failed tests.
So it seems to me that the if statement related to the option should be changed to check if the flags --if or --ifnf=all are included, and if not then go to the else statement on line 378.
Since otherwise it seems that --ifnf=all does nothing in the code right now.
Related code: https://github.com/pytest-dev/pytest/blob/main/src/_pytest/cacheprovider.py#L344

Also sorry, this is my first issue I am working on as a contributor, so I am a bit new to the Pytest codebase, and not really knowledgeable about the functionalities yet.

@DetachHead
Copy link
Contributor Author

DetachHead commented Aug 25, 2023

A PR to update the docs will be great.

Why not make ---lfnf imply --lf?

My understanding is that people often put the --lfnf configuration into their configuration file, so making that change would make all runs --lf.

In that case would it make more sense for lfnf to be an ini option instead of a cli option?

@nicoddemus
Copy link
Member

In that case would it make more sense for lfnf to be an ini option instead of a cli option?

Not necessarily, it is also common to use it via command-line.

@nicoddemus
Copy link
Member

nicoddemus commented Aug 26, 2023

@seanjedi

--if and --ifnf=all do the same exact thing then?

No, --lfnf controls what --lf will do when it encounters the scenario where there are no more failures, which is the case when you are in a change code/run tests/fix code cycle. The default (all), will run all tests then, while none will not run any tests and exit successfully.

(The information above might be helpful to include into the https://docs.pytest.org/en/stable/how-to/cache.html#behavior-when-no-tests-failed-in-the-last-run section for those interested in contribute!).

I think the --lfnf option only exists for backward compatibility, because it makes sense to just say "no more failures" and not run anything (which is the behavior of --lfnf=none) when you reach the point where you fixed all failures: it is cheap to just say that and quit. The user, if they want to run the full suite now, just need to remove the --lf flag then. In other words, since the beginning the behavior has been to run all tests, but when the idea of "run nothing if there are no failures" came up later, we decided to include a new option to add the new behavior (none) to preserve backward compatibility.

@RonnyPfannschmidt
Copy link
Member

I think its time to close this as works ask intended

@nicoddemus
Copy link
Member

Indeed it does, but we have since then redirected it to update/clarify the docs instead.

@nicoddemus
Copy link
Member

@RonnyPfannschmidt is right, better to close this one and create a follow up: #11354

Thanks @seanjedi and @DetachHead!

@nicoddemus nicoddemus closed this as not planned Won't fix, can't repro, duplicate, stale Aug 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: docs documentation improvement, missing or needing clarification
Projects
None yet
Development

No branches or pull requests

4 participants