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

Improved handling of module __path__ attribute for namespace packages, fixes #1321 #1402

Merged
merged 6 commits into from Sep 16, 2018

Conversation

Projects
None yet
3 participants
@daa
Contributor

daa commented Jun 27, 2018

Summary of changes

Improved __path__ attribute handling for namespace packages, fixes a bug revealed with python3.6 when there are 2 packages in the same namespace one installed and one in current working directory.

Closes #1321

Pull Request Checklist

  • Changes have tests
  • News fragment added in changelog.d.
@@ -109,3 +109,34 @@ def test_namespace_package_installed_and_cwd(self, tmpdir):
]
with test.test.paths_on_pythonpath([str(target)]):
subprocess.check_call(pkg_resources_imp, cwd=str(pkg_A))
@pytest.mark.skipif(bool(os.environ.get("APPVEYOR")),

This comment has been minimized.

@pganssle

pganssle Aug 17, 2018

Member

No need for the bool here.

Also, can you make this xfail? This test should pass on Appveyor, so it would be good to know if it changes from xfail to xpass.

This comment has been minimized.

@daa

daa Aug 20, 2018

Contributor

I noticed that in appveyor run for this pull request 3 of 4 namespace tests passed when running under python-2.7.15 and all 4 passed when running under python-3.6.6. Should I remove xfail for them?

This comment has been minimized.

@pganssle

pganssle Aug 20, 2018

Member

@daa The rest of the repo is not super consistent about xfail vs skip. Going forward we're trying to make a clear distinction, and at some point I'll go through and try and clean it up so that we can turn on strict xfail (e.g. if an xfail test succeeds, that's a failure).

This comment has been minimized.

@pganssle

pganssle Aug 20, 2018

Member

Oh sorry, I misunderstood. Yeah, I think you should change these so that you don't introduce any new XPASSes.

This comment has been minimized.

@daa

daa Aug 21, 2018

Contributor

Ok, I removed xfail marks for namespace tests that pass on AppVeyor.

@pytest.mark.xfail(
os.environ.get("APPVEYOR"),
reason="https://github.com/pypa/setuptools/issues/851",
)

This comment has been minimized.

@pganssle

pganssle Aug 21, 2018

Member

I think you can drop the APPVEYOR xfail entirely, since it seems that this test is only failing on Appveyor for Python 2.7, which is covered by the xfail anyway.

This comment has been minimized.

@daa

daa Aug 21, 2018

Contributor

You are right, removed.

@jaraco

This comment has been minimized.

Member

jaraco commented Sep 16, 2018

The diff indicates #851 may be addressed by this change also.

@jaraco

This comment has been minimized.

Member

jaraco commented Sep 16, 2018

Thank you for this.

@jaraco jaraco merged commit 5c8ff5a into pypa:master Sep 16, 2018

5 checks passed

codecov/patch 100% of diff hit (target 81.11%)
Details
codecov/project 81.34% (+0.22%) compared to e9bdeda
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
@daa

This comment has been minimized.

Contributor

daa commented Sep 16, 2018

Thank you for merging.

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