Skip to content

PYTEST_DISABLE_PLUGIN_AUTOLOAD: allow 0 to enable it explicitly#4519

Closed
blueyed wants to merge 1 commit intopytest-dev:featuresfrom
blueyed:PYTEST_DISABLE_PLUGIN_AUTOLOAD-bool
Closed

PYTEST_DISABLE_PLUGIN_AUTOLOAD: allow 0 to enable it explicitly#4519
blueyed wants to merge 1 commit intopytest-dev:featuresfrom
blueyed:PYTEST_DISABLE_PLUGIN_AUTOLOAD-bool

Conversation

@blueyed
Copy link
Contributor

@blueyed blueyed commented Dec 9, 2018

This assumes that only "1" was used previously to enable it.

@blueyed blueyed requested a review from a user December 9, 2018 10:41
@blueyed blueyed changed the base branch from master to features December 9, 2018 10:41
Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

this is a breaking change, its not unthinkable that someone did set it to some text

@blueyed
Copy link
Contributor Author

blueyed commented Dec 9, 2018

Yes, it's for "features" and I've requested a review from @hsoft that introduced it for using it.
I do not assume it is widely used yet.

What do you suggest?
(I think it's unfortunate to not having done if like that from the beginning btw)

@blueyed
Copy link
Contributor Author

blueyed commented Dec 9, 2018

Regarding #4518 I think we could also have a new env variable to match the new config option, where a list of plugins would be specified (that should be loaded), and an empty value would act as PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 now, i.e. would not load any.

@blueyed
Copy link
Contributor Author

blueyed commented Dec 9, 2018

Closing, but would still like to get feedback from @hsoft about how it is used in the wild though, of course.

@blueyed blueyed closed this Dec 9, 2018
@blueyed blueyed deleted the PYTEST_DISABLE_PLUGIN_AUTOLOAD-bool branch December 9, 2018 17:57
@ghost
Copy link

ghost commented Dec 10, 2018

I introduced it for Gentoo where plugin autoloading wreaks havok in our tests (as soon as you install stuff like "pytest-relaxed" or other disrupting plugins like that, you break many unrelated packages). Instead of having to explicitly disable those plugins in all pytest-tested packages, I thought it was a better idea to introduce this option and start using it systematically.

However, pytest moves too fast for us and we haven't stabilized pytest 3.9 yet, so this feature isn't used yet. To my knowledge, changing this feature wouldn't break anything in Gentoo. Therefore, you won't get objections from me if you want to introduce a breaking change. As long as there's an easy way to disable autoloading, I'm happy.

@ghost
Copy link

ghost commented Dec 10, 2018

With regards to this particular PR, I find the semantic of "DISABLE=0" a bit strange. I don't see why anyone would want to set it explicitly (unless the default behavior for pytest autoloading changes, but in this case, we might as well introduce a new variable name)

@blueyed
Copy link
Contributor Author

blueyed commented Dec 10, 2018

Thanks for the feedback.

I wonder why you are not using tox, at least when it comes to pytest itself?

I assume you are running the tests for/while packaging, right?

@ghost
Copy link

ghost commented Dec 10, 2018

Because we already have a multi-python mechanism in place, in which testing takes place, and tox conflicts with it.

I guess that we could theoretically create throw-able virtualenvs for each test run, but it would require some packaging work because we can't use pip install at test time: any internet access during build/install/test represents a QA violation. Portage manages package fetching. It's a trust issue: we can't blindly trust pip/npm/rubygems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants