tests: fix tests that require PYTEST_DISABLE_PLUGIN_AUTOLOAD to be unset

[blueyed]

Fix pytest's own tests with PYTEST_DISABLE_PLUGIN_AUTOLOAD=1.

[blueyed]

Maybe that should be done in a more central place, Testdir.__init__?

[blueyed]

Maybe that should be done in a more central place, Testdir.__init__?

Not in Testdir.__init__ though, since then it would break for other users of pytester again after all.

Therefore I suggest leaving it like this (in this PR), until it eventually gets deprecated / replaced by an env that allows a list (or rather via PYTEST_ADDOPTS=-oload_entrypoint_plugins= (#4521)).

[nicoddemus]

Maybe that should be done in a more central place, Testdir.init?

How about an autouse fixture which unsets this variable?

But I'm not sure, your initial point seems valid, we shouldn't actually leak any external PYTEST_ variable to the underlying test when using Testdir. How about we remove all PYTEST_ variables during Testdir.__init__? I don't see a problem with such a change going to features and properly advertised in the CHANGELOG.

[blueyed]

@nicoddemus
Unsetting the env will break it for other plugins/users relying on this.
I assume this is used in CI on purpose.

Therefore I think it is good to unset it explicitly where we need it (and where we control the entrypoints processing manually mostly).

[nicoddemus]

Unsetting the env will break it for other plugins/users relying on this.
I assume this is used in CI on purpose.

Possibly, but in those cases users might want to adjust their tests accordingly.

Well I'm not sure, we have this variables:

• PYTEST_ADDOPTS
• PYTEST_DEBUG
• PYTEST_PLUGINS
• PYTEST_DISABLE_PLUGIN_AUTOLOAD
• PYTEST_CURRENT_TEST

I don't see a case where a user would like for a test to depend on one of these variables being set externally... if a test does depend on them, the user should set it explicitly in the test, I believe.

Therefore I think it is good to unset it explicitly where we need it (and where we control the entrypoints processing manually mostly).

Problem is that it is error prone: nothing was failing until you executed the suite with PYTEST_DISABLE_PLUGIN_AUTOLOAD set. Nothing prevents us from adding a new test in the future that breaks if PYTEST_DISABLE_PLUGIN_AUTOLOAD is set in the user's system. 😕

[blueyed]

PYTEST_DISABLE_PLUGIN_AUTOLOAD was added in #3787, and it sounds like it is used in distributions - e.g. pytest's test should not use pytest-pdb for example if it is installed also. They could probably use tox, but the whole point of the variable is to control how pytester behaves - and therefore it does not really work to unset/ignore it there.

[nicoddemus]

Sorry, not sure I follow... it seems to me that pytester should never autoload external plugins, as we want to isolate the test run.

[RonnyPfannschmidt]

@nicoddemus that breaks pytester for 3rd party plugins

[nicoddemus]

@nicoddemus that breaks pytester for 3rd party plugins

You mean PYTEST_DISABLE_PLUGIN_AUTOLOAD? I'm proposing removing it from the environment during pytester startup (just to make sure we are on the same page). This ensures the executed tests by pytester won't inherit that environment variable. If that breaks a 3rd party plugin, I would argue it is for the better, as it is a sign they are depending on external environment variables.

Or am I missing something?

[blueyed]

@nicoddemus
PYTEST_DISABLE_PLUGIN_AUTOLOAD was added explicitly to prevent loading of plugins via entrypoints, and therefore also needs to be passed through to internal runs.

I agree with your sentiment of having a clear environment for internal runs, but it does not work in this case.

See #4519 (comment).

[blueyed]

PYTEST_DISABLE_PLUGIN_AUTOLOAD was not added for 3rd party plugins, but for testing pytest (plugins) in a (distribution) CI.

[blueyed]

@nicoddemus
You approved/merged it in #3787.

[nicoddemus]

I see, thanks for the reference.

How about the other environment variables other than PYTEST_DISABLE_PLUGIN_AUTOLOAD, do you feel it would make sense to remove them while executing the subtests with testdir?

[blueyed]

Yeah, I'm in favor of removing them / having a clean env.
Please create a follow-up PR for them/those.