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

[FR] Warn about top-level "tests" package in find_packages() by default #3243

Open
1 task done
mgorny opened this issue Apr 3, 2022 · 3 comments · May be fixed by #3259
Open
1 task done

[FR] Warn about top-level "tests" package in find_packages() by default #3243

mgorny opened this issue Apr 3, 2022 · 3 comments · May be fixed by #3259
Labels
enhancement Needs Triage Issues that need to be evaluated for severity and status.

Comments

@mgorny
Copy link
Contributor

mgorny commented Apr 3, 2022

What's the problem this feature will solve?

I often end up reporting bugs about packages using find_packages() without appropriate exclusion lists, that end up installing top-level test or tests directories. These directories are especially problematic since they're bound to cause file collisions between packages, and given their special use I don't think any specific package should be allowed to "own" them.

Describe the solution you'd like

I think it would be best if setuptools could at least warn if find_packages() ends up including test or tests directories, and suggest the package maintainers to exclude them. Perhaps they could also be skipped by default in some future version.

Alternative Solutions

It could be possible to implicitly skip them immediately. While I honestly don't think it would realistically break anything, I'm not sure if someone isn't actually having some workflow relying on this.

Additional context

No response

Code of Conduct

  • I agree to follow the PSF Code of Conduct
@mgorny mgorny added enhancement Needs Triage Issues that need to be evaluated for severity and status. labels Apr 3, 2022
@drewcassidy
Copy link

The docs imply that test folders should be excluded by default, to make things more confusing

@abravalheri
Copy link
Contributor

abravalheri commented Apr 7, 2022

That is a reasonable feature to implement. Would any of you guys would like to give it a go and provide a PR?

@drewcassidy please notice that the docs you pointed out are for automatic discovery, but the procedure you are using is custom discovery. If you would like to submit a PR to make the docs less confusing that would be very appreciated!

@mgorny
Copy link
Contributor Author

mgorny commented Apr 11, 2022

I'll give it a try today. I guess I'll try to make it warn about all directories that would've been excluded by autodiscovery.

mgorny added a commit to mgorny/setuptools that referenced this issue Apr 11, 2022
Trigger SetuptoolsDeprecationWarning whenever find_packages() finds
a top-level package whose name is found in reserved package name list,
e.g. "tests".  Installing these packages is probably always wrong,
yet it is still a frequent mistake in packages using setuptools.  This
warning should increase the chance of the mistake being caught before
releasing.

Fixes pypa#3243
mgorny added a commit to mgorny/setuptools that referenced this issue Apr 11, 2022
Trigger SetuptoolsDeprecationWarning whenever find_packages() finds
a top-level package whose name is found in reserved package name list,
e.g. "tests".  Installing these packages is probably always wrong,
yet it is still a frequent mistake in packages using setuptools.  This
warning should increase the chance of the mistake being caught before
releasing.

Fixes pypa#3243
mgorny added a commit to mgorny/setuptools that referenced this issue Apr 11, 2022
Trigger SetuptoolsDeprecationWarning whenever find_packages() finds
a top-level package whose name is found in reserved package name list,
e.g. "tests".  Installing these packages is probably always wrong,
yet it is still a frequent mistake in packages using setuptools.  This
warning should increase the chance of the mistake being caught before
releasing.

Fixes pypa#3243
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Needs Triage Issues that need to be evaluated for severity and status.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@mgorny @abravalheri @drewcassidy and others