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

no longer consider properties for fixtures of plugin objects (manages warning triggers) #6898

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 18 additions & 45 deletions doc/en/example/parametrize.rst
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,10 @@ objects, they are still using the default pytest representation:

$ pytest test_time.py --collect-only
=========================== test session starts ============================
platform linux -- Python 3.x.y, pytest-6.x.y, py-1.x.y, pluggy-0.x.y
platform linux -- Python 3.x.y, pytest-5.x.y, py-1.x.y, pluggy-0.x.y
cachedir: $PYTHON_PREFIX/.pytest_cache
rootdir: $REGENDOC_TMPDIR
collected 8 items

<Module test_time.py>
<Function test_timedistance_v0[a0-b0-expected0]>
<Function test_timedistance_v0[a1-b1-expected1]>
Expand Down Expand Up @@ -225,7 +224,7 @@ this is a fully self-contained example which you can run with:

$ pytest test_scenarios.py
=========================== test session starts ============================
platform linux -- Python 3.x.y, pytest-6.x.y, py-1.x.y, pluggy-0.x.y
platform linux -- Python 3.x.y, pytest-5.x.y, py-1.x.y, pluggy-0.x.y
cachedir: $PYTHON_PREFIX/.pytest_cache
rootdir: $REGENDOC_TMPDIR
collected 4 items
Expand All @@ -240,11 +239,10 @@ If you just collect tests you'll also nicely see 'advanced' and 'basic' as varia

$ pytest --collect-only test_scenarios.py
=========================== test session starts ============================
platform linux -- Python 3.x.y, pytest-6.x.y, py-1.x.y, pluggy-0.x.y
platform linux -- Python 3.x.y, pytest-5.x.y, py-1.x.y, pluggy-0.x.y
cachedir: $PYTHON_PREFIX/.pytest_cache
rootdir: $REGENDOC_TMPDIR
collected 4 items

<Module test_scenarios.py>
<Class TestSampleWithScenarios>
<Function test_demo1[basic]>
Expand Down Expand Up @@ -319,11 +317,10 @@ Let's first see how it looks like at collection time:

$ pytest test_backends.py --collect-only
=========================== test session starts ============================
platform linux -- Python 3.x.y, pytest-6.x.y, py-1.x.y, pluggy-0.x.y
platform linux -- Python 3.x.y, pytest-5.x.y, py-1.x.y, pluggy-0.x.y
cachedir: $PYTHON_PREFIX/.pytest_cache
rootdir: $REGENDOC_TMPDIR
collected 2 items

<Module test_backends.py>
<Function test_db_initialized[d1]>
<Function test_db_initialized[d2]>
Expand Down Expand Up @@ -354,30 +351,6 @@ And then when we run the test:

The first invocation with ``db == "DB1"`` passed while the second with ``db == "DB2"`` failed. Our ``db`` fixture function has instantiated each of the DB values during the setup phase while the ``pytest_generate_tests`` generated two according calls to the ``test_db_initialized`` during the collection phase.

Indirect parametrization
---------------------------------------------------

Using the ``indirect=True`` parameter when parametrizing a test allows to
parametrize a test with a fixture receiving the values before passing them to a
test:

.. code-block:: python

import pytest


@pytest.fixture
def fixt(request):
return request.param * 3


@pytest.mark.parametrize("fixt", ["a", "b"], indirect=True)
def test_indirect(fixt):
assert len(fixt) == 3

This can be used, for example, to do more expensive setup at test run time in
the fixture, rather than having to run those setup steps at collection time.

.. regendoc:wipe

Apply indirect on particular arguments
Expand Down Expand Up @@ -416,19 +389,22 @@ The result of this test will be successful:

.. code-block:: pytest

$ pytest -v test_indirect_list.py
$ pytest test_indirect_list.py --collect-only
=========================== test session starts ============================
platform linux -- Python 3.x.y, pytest-6.x.y, py-1.x.y, pluggy-0.x.y -- $PYTHON_PREFIX/bin/python
platform linux -- Python 3.x.y, pytest-5.x.y, py-1.x.y, pluggy-0.x.y
cachedir: $PYTHON_PREFIX/.pytest_cache
rootdir: $REGENDOC_TMPDIR
collecting ... collected 1 item

test_indirect_list.py::test_indirect[a-b] PASSED [100%]
collected 1 item
<Module test_indirect_list.py>
<Function test_indirect[a-b]>

============================ 1 passed in 0.12s =============================
========================== no tests ran in 0.12s ===========================

.. regendoc:wipe

Note, that each argument in `parametrize` list should be explicitly declared in corresponding
python test function or via `indirect`.

Parametrizing test methods through per-class configuration
--------------------------------------------------------------

Expand Down Expand Up @@ -508,11 +484,8 @@ Running it results in some skips if we don't have all the python interpreters in
.. code-block:: pytest

. $ pytest -rs -q multipython.py
ssssssssssss...ssssssssssss [100%]
========================= short test summary info ==========================
SKIPPED [12] multipython.py:29: 'python3.5' not found
SKIPPED [12] multipython.py:29: 'python3.7' not found
3 passed, 24 skipped in 0.12s
........................... [100%]
27 passed in 0.12s

Indirect parametrization of optional implementations/imports
--------------------------------------------------------------------
Expand Down Expand Up @@ -572,15 +545,15 @@ If you run this with reporting for skips enabled:

$ pytest -rs test_module.py
=========================== test session starts ============================
platform linux -- Python 3.x.y, pytest-6.x.y, py-1.x.y, pluggy-0.x.y
platform linux -- Python 3.x.y, pytest-5.x.y, py-1.x.y, pluggy-0.x.y
cachedir: $PYTHON_PREFIX/.pytest_cache
rootdir: $REGENDOC_TMPDIR
collected 2 items

test_module.py .s [100%]

========================= short test summary info ==========================
SKIPPED [1] conftest.py:12: could not import 'opt2': No module named 'opt2'
SKIPPED [1] $REGENDOC_TMPDIR/conftest.py:12: could not import 'opt2': No module named 'opt2'
======================= 1 passed, 1 skipped in 0.12s =======================

You'll see that we don't have an ``opt2`` module and thus the second test run
Expand Down Expand Up @@ -634,7 +607,7 @@ Then run ``pytest`` with verbose mode and with only the ``basic`` marker:

$ pytest -v -m basic
=========================== test session starts ============================
platform linux -- Python 3.x.y, pytest-6.x.y, py-1.x.y, pluggy-0.x.y -- $PYTHON_PREFIX/bin/python
platform linux -- Python 3.x.y, pytest-5.x.y, py-1.x.y, pluggy-0.x.y -- $PYTHON_PREFIX/bin/python
cachedir: $PYTHON_PREFIX/.pytest_cache
rootdir: $REGENDOC_TMPDIR
collecting ... collected 14 items / 11 deselected / 3 selected
Expand Down
67 changes: 5 additions & 62 deletions doc/en/getting-started.rst
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Installation and Getting Started
===================================

**Pythons**: Python 3.5, 3.6, 3.7, 3.8, 3.9, PyPy3
**Pythons**: Python 3.5, 3.6, 3.7, PyPy3

**Platforms**: Linux and Windows

Expand All @@ -28,7 +28,7 @@ Install ``pytest``
.. code-block:: bash

$ pytest --version
pytest 6.0.1
This is pytest version 5.x.y, imported from $PYTHON_PREFIX/lib/python3.7/site-packages/pytest/__init__.py

.. _`simpletest`:

Expand All @@ -53,7 +53,7 @@ That’s it. You can now execute the test function:

$ pytest
=========================== test session starts ============================
platform linux -- Python 3.x.y, pytest-6.x.y, py-1.x.y, pluggy-0.x.y
platform linux -- Python 3.x.y, pytest-5.x.y, py-1.x.y, pluggy-0.x.y
cachedir: $PYTHON_PREFIX/.pytest_cache
rootdir: $REGENDOC_TMPDIR
collected 1 item
Expand All @@ -73,7 +73,7 @@ That’s it. You can now execute the test function:
FAILED test_sample.py::test_answer - assert 4 == 5
============================ 1 failed in 0.12s =============================

The ``[100%]`` refers to the overall progress of running all test cases. After it finishes, pytest then shows a failure report because ``func(3)`` does not return ``5``.
This test returns a failure report because ``func(3)`` does not return ``5``.

.. note::

Expand Down Expand Up @@ -112,15 +112,9 @@ Execute the test function with “quiet” reporting mode:
. [100%]
1 passed in 0.12s

.. note::

The ``-q/--quiet`` flag keeps the output brief in this and following examples.

Group multiple tests in a class
--------------------------------------------------------------

.. regendoc:wipe

Once you develop multiple tests, you may want to group them into a class. pytest makes it easy to create a class containing more than one test:

.. code-block:: python
Expand Down Expand Up @@ -159,61 +153,10 @@ Once you develop multiple tests, you may want to group them into a class. pytest

The first test passed and the second failed. You can easily see the intermediate values in the assertion to help you understand the reason for the failure.

Grouping tests in classes can be beneficial for the following reasons:

* Test organization
* Sharing fixtures for tests only in that particular class
* Applying marks at the class level and having them implicitly apply to all tests

Something to be aware of when grouping tests inside classes is that each test has a unique instance of the class.
Having each test share the same class instance would be very detrimental to test isolation and would promote poor test practices.
This is outlined below:

.. regendoc:wipe

.. code-block:: python

# content of test_class_demo.py
class TestClassDemoInstance:
def test_one(self):
assert 0

def test_two(self):
assert 0


.. code-block:: pytest

$ pytest -k TestClassDemoInstance -q
FF [100%]
================================= FAILURES =================================
______________________ TestClassDemoInstance.test_one ______________________

self = <test_class_demo.TestClassDemoInstance object at 0xdeadbeef>

def test_one(self):
> assert 0
E assert 0

test_class_demo.py:3: AssertionError
______________________ TestClassDemoInstance.test_two ______________________

self = <test_class_demo.TestClassDemoInstance object at 0xdeadbeef>

def test_two(self):
> assert 0
E assert 0

test_class_demo.py:6: AssertionError
========================= short test summary info ==========================
FAILED test_class_demo.py::TestClassDemoInstance::test_one - assert 0
FAILED test_class_demo.py::TestClassDemoInstance::test_two - assert 0
2 failed in 0.12s

Request a unique temporary directory for functional tests
--------------------------------------------------------------

``pytest`` provides `Builtin fixtures/function arguments <https://docs.pytest.org/en/stable/builtin.html>`_ to request arbitrary resources, like a unique temporary directory:
``pytest`` provides `Builtin fixtures/function arguments <https://docs.pytest.org/en/latest/builtin.html>`_ to request arbitrary resources, like a unique temporary directory:

.. code-block:: python

Expand Down
22 changes: 11 additions & 11 deletions doc/en/writing_plugins.rst
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ Plugin discovery order at tool startup
your ``conftest.py`` file in the top level test or project root directory.

* by recursively loading all plugins specified by the
:globalvar:`pytest_plugins` variable in ``conftest.py`` files
``pytest_plugins`` variable in ``conftest.py`` files


.. _`pytest/plugin`: http://bitbucket.org/pytest-dev/pytest/src/tip/pytest/plugin/
Expand Down Expand Up @@ -227,7 +227,7 @@ import ``helper.py`` normally. The contents of
Requiring/Loading plugins in a test module or conftest file
-----------------------------------------------------------

You can require plugins in a test module or a ``conftest.py`` file using :globalvar:`pytest_plugins`:
You can require plugins in a test module or a ``conftest.py`` file like this:

.. code-block:: python

Expand All @@ -241,31 +241,31 @@ application modules:

pytest_plugins = "myapp.testsupport.myplugin"

:globalvar:`pytest_plugins` are processed recursively, so note that in the example above
if ``myapp.testsupport.myplugin`` also declares :globalvar:`pytest_plugins`, the contents
``pytest_plugins`` variables are processed recursively, so note that in the example above
if ``myapp.testsupport.myplugin`` also declares ``pytest_plugins``, the contents
of the variable will also be loaded as plugins, and so on.

.. _`requiring plugins in non-root conftests`:

.. note::
Requiring plugins using :globalvar:`pytest_plugins` variable in non-root
Requiring plugins using a ``pytest_plugins`` variable in non-root
``conftest.py`` files is deprecated.

This is important because ``conftest.py`` files implement per-directory
hook implementations, but once a plugin is imported, it will affect the
entire directory tree. In order to avoid confusion, defining
:globalvar:`pytest_plugins` in any ``conftest.py`` file which is not located in the
``pytest_plugins`` in any ``conftest.py`` file which is not located in the
tests root directory is deprecated, and will raise a warning.

This mechanism makes it easy to share fixtures within applications or even
external applications without the need to create external plugins using
the ``setuptools``'s entry point technique.

Plugins imported by :globalvar:`pytest_plugins` will also automatically be marked
Plugins imported by ``pytest_plugins`` will also automatically be marked
for assertion rewriting (see :func:`pytest.register_assert_rewrite`).
However for this to have any effect the module must not be
imported already; if it was already imported at the time the
:globalvar:`pytest_plugins` statement is processed, a warning will result and
``pytest_plugins`` statement is processed, a warning will result and
assertions inside the plugin will not be rewritten. To fix this you
can either call :func:`pytest.register_assert_rewrite` yourself before
the module is imported, or you can arrange the code to delay the
Expand Down Expand Up @@ -430,9 +430,9 @@ additionally it is possible to copy examples for an example folder before runnin

$ pytest
=========================== test session starts ============================
platform linux -- Python 3.x.y, pytest-6.x.y, py-1.x.y, pluggy-0.x.y
platform linux -- Python 3.x.y, pytest-5.x.y, py-1.x.y, pluggy-0.x.y
cachedir: $PYTHON_PREFIX/.pytest_cache
rootdir: $REGENDOC_TMPDIR, configfile: pytest.ini
rootdir: $REGENDOC_TMPDIR, inifile: pytest.ini
collected 2 items

test_example.py .. [100%]
Expand All @@ -442,7 +442,7 @@ additionally it is possible to copy examples for an example folder before runnin
$REGENDOC_TMPDIR/test_example.py:4: PytestExperimentalApiWarning: testdir.copy_example is an experimental api that may change over time
testdir.copy_example("test_example.py")

-- Docs: https://docs.pytest.org/en/stable/warnings.html
-- Docs: https://docs.pytest.org/en/latest/warnings.html
======================= 2 passed, 1 warning in 0.12s =======================

For more information about the result object that ``runpytest()`` returns, and
Expand Down
5 changes: 5 additions & 0 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -1632,6 +1632,11 @@ def parsefactories(
for name in dir(holderobj):
# The attribute can be an arbitrary descriptor, so the attribute
# access below can raise. safe_getatt() ignores such exceptions.
# additionally properties are ignored by default to avoid triggering warnings
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference, the safe_getattr() and comment above was added in 3a0a0c2 to fix #2234. There I wrote

A better solution would be to completely skip access to all custom descriptors, such that the offending code doesn't even trigger. However I think this requires manually going through the instance and all of its MRO for each and every attribute checking if it might be a proper fixture before accessing it. So I took the easy route here.

So this at least seems to be a step in the right direction. But this was a long time ago.

There's a question if this is any backward compatibility concern here. Is it possible in any way to use a property as a fixture? (I'm not familiar with this code yet).

In any case I think it would be to discuss this change in a separate PR so it can be considered properly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as a breaking change its ok to drop consideration for properties

as methods are descriptors, we cant drop everything

i would prefer if we could set up better separation for fixture lokup

if not isinstance(holderobj, type) and isinstance(
safe_getattr(holderobj.__class__, name, None), property
):
continue
obj = safe_getattr(holderobj, name, None)
marker = getfixturemarker(obj)
if not isinstance(marker, FixtureFunctionMarker):
Expand Down
19 changes: 19 additions & 0 deletions testing/example_scripts/fixtures/plugin_properties/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import warnings

import pytest


class FunPlugin:
@property
def shouldnt_warn(self):
warnings.warn("i_shouldnt_happen")
print("if no warning but thus, then all is bad")

@pytest.fixture
def fix(self):
pass


def pytest_configure(config):
warnings.simplefilter("always", DeprecationWarning)
config.pluginmanager.register(FunPlugin(), "fun")
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
def test_fun(fix):
pass
8 changes: 6 additions & 2 deletions testing/python/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -1350,8 +1350,7 @@ def test_printer_2(self):

class TestFixtureManagerParseFactories:
@pytest.fixture
def testdir(self, request):
testdir = request.getfixturevalue("testdir")
def testdir(self, testdir):
testdir.makeconftest(
"""
import pytest
Expand Down Expand Up @@ -1572,6 +1571,11 @@ def test_collect_custom_items(self, testdir):
result = testdir.runpytest("foo")
result.stdout.fnmatch_lines(["*passed*"])

def test_plugin_properties_are_skipped(self, testdir):
testdir.copy_example("fixtures/plugin_properties")
result = testdir.runpytest("-s", "-k", "fun")
result.stdout.fnmatch_lines(["*passed*"])


class TestAutouseDiscovery:
@pytest.fixture
Expand Down