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

test/test*.py vs. tests/test*.py #187

Closed
DimitriPapadopoulos opened this issue Nov 20, 2022 · 4 comments · Fixed by #189
Closed

test/test*.py vs. tests/test*.py #187

DimitriPapadopoulos opened this issue Nov 20, 2022 · 4 comments · Fixed by #189

Comments

@DimitriPapadopoulos
Copy link
Contributor

While I do not mind the Python Packaging User Guide referring to either test/ or tests/ or even a random word as an example of a directory that contains tests, I believe actual code should support tests/ in addition to test/:

def _add_defaults_optional(self):
optional = ['test/test*.py', 'setup.cfg']

I think most packages out there use tests/.

See pypa/packaging.python.org#1165 and before that codespell-project/codespell#2592 (comment).

@jaraco
Copy link
Member

jaraco commented Nov 24, 2022

I'm not sure that test/test*.py is a good default here. It excludes files that don't start with test (i.e. conftest.py) but includes files that do (i.e. tester_notes.py). And as you've pointed out, it only honors a test directory and not tests. Maybe the default should be test*/**/* (everything in any subdirectory of any directory that begins with test). Or maybe the default should be removed altogether (making no inference about which files should be included in sdist).

In my projects, I rely on setuptools_scm to add every file under source code management, so I don't deal with defaults or implicit overrides. I do as a matter of course exclude tests from packages to install.

My instinct is to remove the default altogether, except for the backward-compatibility concerns that could raise. We could, through a deprecation process, wean projects off of reliance on this default. Since it has to do with sdist generation, it's probably not something that needs extended backward compatibility (old sdists are unaffected by any change).

@DimitriPapadopoulos
Copy link
Contributor Author

Yes, backward compatibility is a concern, and changing test/test*.pytest*/**/* feels better than removing default values. But then I agree, default values have their issues too: what happens, for example, in the case of a Python package named testimony?

Whatever changes here, the Python Packaging User Guide should be updated at the same time:

When building a source distribution for your package, by default only a minimal set of files are included. You may find yourself wanting to include extra files in the source distribution, such as [...] a directory of data files used for testing purposes.
[...]
The following files are included in a source distribution by default:

  • [...]
  • all files matching the pattern test/test*.py

@DimitriPapadopoulos
Copy link
Contributor Author

I do as a matter of course exclude tests from packages to install.

Not that I necessarily disagree, but it is not a matter of course. The Python Packaging User Guide currently implies that tests should be included in source distributions (see excerpts above), and the consensus seems to be that tests should not be included in binary distributions. Therefore:

  • Could you point me to sources of information on consensus on the subject? That would be very helpful for confused end-users - like me.
  • Should such a consensus be documented in the Python Packaging User Guide, as a suggestion more than a necessity?
  • If the consensus is to include test in source distributions but not in binary distributions, how do I achieve that? I cannot find documentation on the subject.

@yukihiko-shinoda
Copy link

Shall we open this issue again? (or open another issue?)
I'm surprised that some of test files were suddenly included in sdist in last build. And it's not complete, followings are lacked:

  • conftest.py
  • Test libraries
  • Test data

I thought that it must be bug.

The remains of issues are:

  • Update Python Packaging User Guide
  • Include test/* and tests/* (not include other like testimony)
  • Discuss whether test code really required or not in sdist

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 a pull request may close this issue.

3 participants