Skip to content

pytester configuration leaks in main pytest configuration #4495

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

Closed
4 tasks done
youtux opened this issue Dec 2, 2018 · 19 comments
Closed
4 tasks done

pytester configuration leaks in main pytest configuration #4495

youtux opened this issue Dec 2, 2018 · 19 comments
Labels
type: bug problem that needs to be addressed

Comments

@youtux
Copy link

youtux commented Dec 2, 2018

I discover this while debugging a failure in pytest-bdd (see pytest-dev/pytest-bdd#276).
It seems that the ini configuration created by testdir.makeini function leaks in the main pytest configuration.
Here it is a test that reproduce the issue:

$ pytest test_bug.py::test_1 test_bug.py::test_2
# test_bug.py
import pytest

pytest_plugins = "pytester"


def test_1(testdir):
    """Creating some configuration in the testdir. We set the `minversion` option, but it can be any option."""
    testdir.makeini(
        """
        [pytest]
        minversion=3.0.0
    """
    )

    testdir.makepyfile(
        """
        import pytest

        def test_expected_minversion():
            assert pytest.config.getini('minversion') == '3.0.0'
    """
    )

    result = testdir.runpytest()
    result.assert_outcomes(passed=1)


def test_2():
    """The configuration made in test_1 should not leak here."""
    assert pytest.config.getini('minversion') != '3.0.0'

This is the configuration I am using:

Package        Version
-------------- -------
atomicwrites   1.2.1
attrs          18.2.0
more-itertools 4.3.0
pip            18.1
pluggy         0.8.0
py             1.7.0
pytest         4.0.1
setuptools     40.6.2
six            1.11.0
wheel          0.32.3

Python 3.7.1 on Mac OS 10.14.1

  • Include a detailed description of the bug or suggestion
  • pip list of the virtual environment you are using
  • pytest and operating system versions
  • Minimal example if possible
@RonnyPfannschmidt RonnyPfannschmidt added the type: bug problem that needs to be addressed label Dec 2, 2018
@RonnyPfannschmidt
Copy link
Member

its not so much leaking, but rather a rather massive/painful bug about pytests internal state

as pytest does in fact patch the object pytest.config, but never undoes the patch

@RonnyPfannschmidt
Copy link
Member

pytest/src/_pytest/main.py

Lines 170 to 172 in ecc5c84

def pytest_configure(config):
__import__("pytest").config = config # compatibility

@RonnyPfannschmidt
Copy link
Member

also ref #3050

@youtux
Copy link
Author

youtux commented Dec 2, 2018

It would be quite unfortunate if pytest.config is removed, since pytest-bdd requires it at test import time (cannot just switch to request.config).

@RonnyPfannschmidt
Copy link
Member

@youtux why would it need it that way?

@youtux
Copy link
Author

youtux commented Dec 2, 2018

Because pytest-bdd generates test items from .feature files at import time, but in order to parse the .feature files it needs to know where they are stored. This value is currently taken from the ini configuration of pytest.
Before pytest-dev/pytest-bdd#255, pytest-bdd used to get this value by calling a fixture, but since fixture cannot be called directly anymore, we changed it to use the pytest.config.getini().

@RonnyPfannschmidt
Copy link
Member

all that happens past pytest-configure time, just configure your plugin at that time instead of getting the globals later

@youtux
Copy link
Author

youtux commented Dec 2, 2018

I don't really get what you are suggesting, @RonnyPfannschmidt

@asottile
Copy link
Member

asottile commented Dec 3, 2018

does this become "wontfix" given the resolution of #3050 (deprecating pytest.config global)?

@RonnyPfannschmidt
Copy link
Member

@asottile we can debate on whether we want pytester to remember/reset the value

@sliwinski-milosz
Copy link

all that happens past pytest-configure time, just configure your plugin at that time instead of getting the globals later

@RonnyPfannschmidt could you please tell us what you meant? Please give us some hint or link to the documentation.
Does the bug only affects pytest.config? Does it mean that if we will switch to solution mentioned by you we will not be affected by it anymore?

Because of the problem with pytest.config one pytest-bdd test is failing and that blocks our releases. I would like to fix it asap.

@RonnyPfannschmidt
Copy link
Member

@sliwinski-milosz to fix pytest-bdd, the pytester plugin would have t fix the reference - note that pytest.config will go away with the next major release

@sliwinski-milosz
Copy link

I think this bug is even worse. It is not only about undoing the patch of the config. If I understand correctly hooks are affected too.

Lets say for now that we configure our plugin using pytest_configure hook.

Please take a look at following steps:

  1. We run our test suite that has one normal test and one test that uses testdir plugin.
  2. We start from test that uses testdir.
  3. When we run testdir.runpytest it runs pytest_configure hook and it configures our plugin with some custom config.
  4. Now we switch to normal test. But it will not run pytest_configure again (and other hooks that are meant to be started once) so still our internal state is broken even if we undone config patch.

Important part is that the testdir test is testing config.

One solution for above is to run our testdir test in subprocess by using --runpytest=subprocess option which guarantees test separation when we try to test config values. But that means those tests will run slower.

@RonnyPfannschmidt
Copy link
Member

@sliwinski-milosz for correctness you need pytest_configure and pytest_unconfigure paired

@sliwinski-milosz
Copy link

Ah ok. In our case unconfigure means bring previous values back.
To do that I need to be able to access the "previous" config. Can I get it from somewhere?
Would be great if it would be passed as an argument to pytest_unconfigure but as far as I can see it provides current config.

@RonnyPfannschmidt
Copy link
Member

@sliwinski-milosz with the current layering of pytest, you have to track it yourself, its quite a pain,

@sliwinski-milosz
Copy link

sliwinski-milosz commented Dec 8, 2018

Thanks! All clear now!

I have opened pull request for pytest-bdd in which I use config stack. It seems that it works fine :)

@RonnyPfannschmidt
Copy link
Member

@sliwinski-milosz great job

@RonnyPfannschmidt
Copy link
Member

Closing as we removed the config global

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug problem that needs to be addressed
Projects
None yet
Development

No branches or pull requests

4 participants