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

fix logic if importlib_metadata.PathDistribution.files is None [breaks pytest 4.6.0|1|2] #5389

Merged
merged 3 commits into from Jun 4, 2019

Conversation

Projects
None yet
5 participants
@dirk-thomas
Copy link
Contributor

commented Jun 4, 2019

For some Python packages the files attribute of a importlib_metadata.PathDistribution is None (see e.g ros2/ros2_documentation#271). In that case the code modified in #5358 and released in 4.6 fails with the stacktrace mentioned in the referenced ticket.

This patch certainly fixes the exception (trying to iterate None). I am not certain if it is the right way to address the issue though.

@codecov

This comment has been minimized.

Copy link

commented Jun 4, 2019

Codecov Report

Merging #5389 into master will decrease coverage by 0.26%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #5389      +/-   ##
=========================================
- Coverage   92.96%   92.7%   -0.27%     
=========================================
  Files         114     114              
  Lines       25493   25493              
  Branches     2480    2480              
=========================================
- Hits        23700   23633      -67     
- Misses       1468    1533      +65     
- Partials      325     327       +2
Impacted Files Coverage Δ
src/_pytest/config/__init__.py 93.68% <ø> (ø) ⬆️
testing/python/show_fixtures_per_test.py 17.64% <0%> (-82.36%) ⬇️
testing/python/setup_plan.py 20% <0%> (-80%) ⬇️
testing/python/metafunc.py 73.93% <0%> (-20.28%) ⬇️
testing/test_warnings.py 44.51% <0%> (-10.37%) ⬇️
src/_pytest/python.py 86.43% <0%> (-5.38%) ⬇️
src/_pytest/fixtures.py 96.7% <0%> (-0.83%) ⬇️
testing/python/fixtures.py 92.27% <0%> (+0.56%) ⬆️
src/_pytest/setuponly.py 95.74% <0%> (+4.25%) ⬆️
testing/python/raises.py 94.2% <0%> (+6.52%) ⬆️
... and 3 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 6a6b6d8...883db6a. Read the comment docs.

@dirk-thomas

This comment has been minimized.

Copy link
Contributor Author

commented Jun 4, 2019

It would be great if this fix could be released in a patch release soon.

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member

commented Jun 4, 2019

this needs a test, however it looks like this can only happen for broken distributions

@jaraco as far as i can tell its a result of read_text in combination with the return value and transform_value(...) patter in importlib_resources

@dirk-thomas

This comment has been minimized.

Copy link
Contributor Author

commented Jun 4, 2019

Can you elaborate on what you mean with "broken distribution"? We were able to boil it down to the package causing it (colcon-package-information) and I would be happy to fix whatever is broken in that package.

@nicoddemus

This comment has been minimized.

Copy link
Member

commented Jun 4, 2019

this needs a test, however it looks like this can only happen for broken distributions

Agreed, but I also think the fix is harmless on our side so we should try to get it in. 👍

@nicoddemus

This comment has been minimized.

Copy link
Member

commented Jun 4, 2019

Added a quick test which reproduces the issue on master and is fixed by this PR. 👍

@nicoddemus nicoddemus merged commit 79ef048 into pytest-dev:master Jun 4, 2019

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
WIP Ready for review
Details
pytest-CI #20190604.7 succeeded
Details

nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Jun 4, 2019

nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Jun 4, 2019

nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Jun 5, 2019

@nicoddemus

This comment has been minimized.

Copy link
Member

commented Jun 5, 2019

Backport: #5401

asottile added a commit that referenced this pull request Jun 5, 2019

@asottile asottile added backported and removed needs backport labels Jun 5, 2019

duncanmmacleod added a commit to duncanmmacleod/gwpy that referenced this pull request Jun 10, 2019

requirements-test: ignore incompatible pytests
pytest-4.6-[012] are incompatible with older setuptools, see pytest-dev/pytest#5389
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.