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

pytester: do not use outer plugins #4518

Closed
wants to merge 2 commits into from

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Dec 9, 2018

Sets PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 by default.

Fixes #4351.

Sets PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 by default.

Fixes pytest-dev#4351.
@codecov
Copy link

codecov bot commented Dec 9, 2018

Codecov Report

Merging #4518 into master will decrease coverage by 53.43%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #4518       +/-   ##
===========================================
- Coverage   95.91%   42.48%   -53.44%     
===========================================
  Files         111       92       -19     
  Lines       25093    20852     -4241     
  Branches     2448     2337      -111     
===========================================
- Hits        24068     8859    -15209     
- Misses        724    11404    +10680     
- Partials      301      589      +288
Flag Coverage Δ
#docs 29.21% <100%> (-0.8%) ⬇️
#doctesting 29.21% <100%> (-0.8%) ⬇️
#linting 29.21% <100%> (-0.8%) ⬇️
#linux 42.48% <100%> (-53.27%) ⬇️
#nobyte ?
#numpy 40.93% <100%> (-52.54%) ⬇️
#pexpect 40.93% <100%> (-0.95%) ⬇️
#py27 40.93% <100%> (-53.13%) ⬇️
#py34 ?
#py35 ?
#py36 ?
#py37 ?
#trial 40.93% <100%> (-52.54%) ⬇️
#windows ?
#xdist ?
Impacted Files Coverage Δ
src/_pytest/pytester.py 64.67% <100%> (-22.78%) ⬇️
testing/test_pastebin.py 0% <0%> (-100%) ⬇️
testing/test_resultlog.py 0% <0%> (-100%) ⬇️
testing/test_entry_points.py 0% <0%> (-100%) ⬇️
testing/test_doctest.py 0% <0%> (-100%) ⬇️
testing/test_runner_xunit.py 0% <0%> (-100%) ⬇️
testing/test_pathlib.py 0% <0%> (-100%) ⬇️
testing/test_nodes.py 0% <0%> (-100%) ⬇️
testing/deprecated_test.py 0% <0%> (-100%) ⬇️
testing/test_stepwise.py 0% <0%> (-100%) ⬇️
... and 91 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 818aa4d...0b0770d. Read the comment docs.

_, dom = runandparse(testdir, "-n2")
# XXX: why does "-p xdist" work here, but xdist.plugin is required with
# other tests (to recognize the pytest_configure hook in there)?!
_, dom = runandparse(testdir, "-p xdist -n2")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any idea about this?
I've assumed that -p xdist would work in general, but it looks like if it was loaded in the outer pytest already xdist does not contain the pytest_configure hook - likely because the entrypoint mapping is not considered anymore (from "xdist" to "xdist.plugin") - but why does it work here then?

Copy link
Member

Choose a reason for hiding this comment

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

the -p argument passes modules, not entrypoints, ever since xdist was split up to aid removing messy stuff that was no longer usable in that way

the xdist module itself no longer contains the hooks, different submodules do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah.. but why is "-p xdist" enough here?

Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/pytest-dev/pytest-xdist/blob/master/setup.py#L19 - it appears the xdist core plugin is named by entrypoint

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What confuses me here is that -p xdist.plugin is required in other places here, but -p xdist here. Likely some internal sys.path thing or similar..

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.

i propose introducing a pytest ini option and a mark for white-listing distributions, for pytester
and triggering a warning in pytester if it is not set (and then just not checking it)

simply disabling it certainly doesn't cut it

@blueyed
Copy link
Contributor Author

blueyed commented Dec 9, 2018

i propose introducing a pytest ini option and a mark for white-listing distributions, for pytester
and triggering a warning in pytester if it is not set (and then just not checking it)

Not sure if I understand this correctly: if the ini option / mark is not provided it should issue a warning, and otherwise load the whitelisted/given plugins (which could be empty then)?
So plugins using pytester are supposed to add the option/mark to get rid of the warning then?

I think it could be also just silent, but allow for pytest to use the option itself?

@RonnyPfannschmidt
Copy link
Member

the ini option/mark would set the elements of the whitelist for plugin loading,

an empty whitelist must trigger a warning and load all plugins, else we do have a breaking change that will break testsuites of other pytest plugins

@blueyed
Copy link
Contributor Author

blueyed commented Dec 9, 2018

an empty whitelist must trigger a warning

I would consider empty different from not set here, no?

@RonnyPfannschmidt
Copy link
Member

i would consider them the same, as practically speaking not using the whitelist is for certain to make a flaky testsuite

@blueyed
Copy link
Contributor Author

blueyed commented Dec 9, 2018

Assuming there would be an option/marker - how would we set it for all of pytest's own tests then?
Not using an env makes it even more difficult to get it into inner test runs.
I've some POC to use load_entrypoint_plugins=pytester.

@RonnyPfannschmidt
Copy link
Member

pytests own tests should use pytests own pytest.ini

@blueyed
Copy link
Contributor Author

blueyed commented Dec 9, 2018

pytests own tests should use pytests own pytest.ini

That would include things like testpaths etc then though.
If going that route it would be better to have a fixture with a default config to be used then.

@blueyed
Copy link
Contributor Author

blueyed commented Dec 9, 2018

Closing in favor of #4521.

@blueyed blueyed closed this Dec 9, 2018
@blueyed blueyed deleted the pytest-no-outer-plugins branch December 9, 2018 14:12
blueyed added a commit to blueyed/pytest that referenced this pull request Feb 2, 2020
blueyed added a commit to blueyed/pytest that referenced this pull request Feb 2, 2020
blueyed added a commit to blueyed/pytest that referenced this pull request Feb 2, 2020
blueyed added a commit to blueyed/pytest that referenced this pull request Feb 2, 2020
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.

None yet

2 participants