improve(fixtures-per-test): exclude pseudo fixtures from output #11295 #12129
Conversation
fb25685 to
c0a23ac
Compare
|
Issue #12086 suggests another possible benefit of removing pseudo fixtures from the @pytest.fixture
def user_defined(request):
return request.param * 3
@pytest.mark.parametrize('user_defined', [1])
def test_function(user_defined):
assert user_defined == 3This test will fail: Yet the current output of But if pseudo fixtures are excluded from the output, there is a stronger message about the user-defined fixtures's non-use because no fixture use will be reported. Documentation needs to clarifyFrom one point of view, one mystery has been replaced with another, so I definitely think documentation needs to be available to help clarify the matter for new users. Devil's advocateShould |
src/_pytest/python.py
Outdated
| if verbose <= 0 and argname.startswith("_"): | ||
| return |
There was a problem hiding this comment.
I believe this condition needs to be moved into write_item before the occurrence of the terminal writes
tw.sep("-", f"fixtures used by {item.name}")
tw.sep("-", f"({get_best_relpath(item.function)})") and adapted to filter fixturedefs of such private fixtures (assuming verbosity less than or equal to zero).
Otherwise there is a circumstance where all an item's fixtures have been excluded from the output yet the terminal still starts being written to as if fixture output is about to follow.
For example, if all of test_private_fixtures's fixtures start with _ and verbosity is zero or less, then no fixtures will be shown but fixtures used by test_private_fixtures. . . etc will be shown.
TODO:
- add tests for fixtures starting with
_ - adapt condition to conditionally filter the
fixturedefslist
I could be wrong about all of this. I will write up the test tonight or tomorrow and see.
There was a problem hiding this comment.
I have confirmed that in this PR private fixtures are not shown when verbosity is zero or less but the fixtures used by . . . is still printed. Is this desired behaviour?
I don't know. It is how pytest currently handles reporting on private fixtures.
import pytest
# DEFINE FIXTURES
#################
@pytest.fixture
def _private():
"""Private fixture"""
pass
@pytest.fixture
def public():
"""Public fixture."""
pass
# TESTS
#################
def test_private_fixture(_private):
pass
def test_public_fixture(public):
pass
@pytest.mark.parametrize("pseudo", [1])
def test_pseudo_fixture(pseudo):
pass
def test_no_fixture():
passRunning pytest --fixtures-per-test on the current version of pytest shows:
----------- fixtures used by test_private_fixture ------------
-------------------- (test_private.py:14) --------------------
------------ fixtures used by test_public_fixture ------------
-------------------- (test_private.py:17) --------------------
public -- test_private.py:9
Public fixture.
---------- fixtures used by test_pseudo_fixture[1] -----------
-------------------- (test_private.py:20) --------------------
pseudo -- .../_pytest/python.py:1112
no docstring available
I'm proposing it shows:
--------------------- fixtures used by test_public_fixture ----------------------
-------------------------- (test_fixture_marker.py:23) --------------------------
public -- test_fixture_marker.py:12
Public fixture.
Notable differences:
- pseudo fixtures in tests are ignored
- a test with only private fixtures is treated like a test with no fixtures at verbosity zero or less (i.e. the test is not reported on at all)
The output this PR currently produces is:
--------------------- fixtures used by test_private_fixture ---------------------
-------------------------- (test_fixture_marker.py:20) --------------------------
--------------------- fixtures used by test_public_fixture ----------------------
-------------------------- (test_fixture_marker.py:23) --------------------------
public -- test_fixture_marker.py:12
Public fixture.
bluetech
left a comment
There was a problem hiding this comment.
Thanks for the PR @WarrenTheRabbit. IMO it is indeed better not to show params as fixtures in --fixtures-per-test output, for the reasons you stated, and also because they are somewhat already reflected in the test id (the part in [...]).
For the implementation you've used _get_direct_parametrize_args to exclude the params. But at this point we've already gathered this info for the item (test), which can be found in the callspec field. So it's better to use that.
However, I'd even go a step further and just exclude "pseudo" fixtures directly. This would involve adding an indication on the FixtureDef whether it's a "pseudo" fixture (we really ought to find a better name for this...). As long as it's only used for "information" purposes such as --fixtures-per-test and not for the actual business logic of fixtures, I have no objection to adding such an indication. Then, the change could be to just skip over such FixtureDefs.
…zation helper fixtures No functional changes. This is intended for next commit (wants to skip showing these fixtures in `--fixtures-per-test`). These fixtures were previously called "pseudo fixtures". I took the opportunity to rename them "direct param fixture def", which, while less catchy, is more self-descriptive and less judgmental.
ba38c1b to
4496372
Compare
bluetech
left a comment
There was a problem hiding this comment.
- Rebased
- Implemented my suggestion above to use a separate
DirectParamFixtureDefclass in order to detect these fixtures - Made various minor tweaks to the implementation
So should be good to go now.
Fix pytest-dev#11295 by excluding from the --fixtures-per-test output any 'pseudo fixture' that results from directy parametrizing a test e.g. with ``@pytest.mark.parametrize``. The justification for removing these fixtures from the report is that a) They are unintuitive. Their appearance in the fixtures-per-test report confuses new users because the fixtures created via ``@pytest.mark.parametrize`` do not confrom to the expectations established in the documentation; namely, that fixtures are - richly reusable - provide setup/teardown features - created via the ``@pytest.fixture` decorator b) They are an internal implementation detail. It is not the explicit goal of the direct parametrization mark to create a fixture; instead, pytest's internals leverages the fixture system to achieve the explicit goal: a succinct batch execution syntax. Consequently, exposing the fixtures that implement the batch execution behaviour reveal more about pytest's internals than they do about the user's own design choices and test dependencies.
4496372 to
eff72f7
Compare
Closes #11295 by excluding 'pseudo fixtures' from the
--fixtures-per-testoutput.For example, when
pytest --fixtures-per-testis run, this testwill no longer produce output such as this:
Instead, it will be:
Justification for changing the output
The original output did not match with new users' intuitions and expectations
As a new user, I found it unintuitive to see the
@pytest.mark.parametrizevariables appear in my--fixtures-per-testreport. I am of the opinion that the inclusion of pseudo fixtures in the output confuses new users because they do not conform to the expectations established in the documentation. Namely, that fixtures are@pytest.fixturedecoratorThe original output puts attention on internal implementation details
The purpose of
--fixtures-per-testis to create a summary of the user's fixture decisions and dependencies. Yet creating a fixture for the user is not the goal of the direct parametrization mark. Instead, _pytest'_s internals just leverage the fixture system to achieve the actual goal: a succinct batch execution syntax. I believe that including the pseudo fixtures in the output exposes internal implementation details unnecessarily and distracts from the user-side summary.Checklist
closes #XYZWto the PR . . .AUTHORSin alphabetical order.