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

sdist is missing tests #6294

Open
mtelka opened this issue Oct 30, 2024 · 15 comments
Open

sdist is missing tests #6294

mtelka opened this issue Oct 30, 2024 · 15 comments

Comments

@mtelka
Copy link

mtelka commented Oct 30, 2024

The sdist package at PyPI is missing tests. Please add tests to sdist to make downstream testing easier. Thank you.

@matteius
Copy link
Member

@mtelka to be honest, I am not sure what is being requested here.

@mtelka
Copy link
Author

mtelka commented Oct 30, 2024

Please try this (sdist at PyPI):

$ wget https://files.pythonhosted.org/packages/source/p/pipenv/pipenv-2024.3.0.tar.gz
$ tar xf pipenv-2024.3.0.tar.gz
$ ls pipenv-2024.3.0/tests
ls: cannot access 'pipenv-2024.3.0/tests': No such file or directory
$

And now the same with the github tarball:

$ wget https://github.com/pypa/pipenv/archive/refs/tags/v2024.3.0.tar.gz
$ tar xf v2024.3.0.tar.gz
$ ls pipenv-2024.3.0/tests
conftest.py  fixtures  __init__.py  integration  pypi  test_artifacts  unit
$

@matteius
Copy link
Member

matteius commented Oct 30, 2024

We intentionally prune out the tests from the pypi published wheels and packages, that would be a very large package size if we did not.

@mtelka
Copy link
Author

mtelka commented Oct 30, 2024

I agree that tests should not be in wheels, but sdists are different. Most Python projects keep tests in their sdists. Also setuptools does so by default.

@matteius matteius added the Contributor Candidate The issue has been identified/triaged and contributions are welcomed/encouraged. label Nov 6, 2024
@oz123
Copy link
Contributor

oz123 commented Dec 11, 2024

First , this issue revealed to me that we are not distributing wheels.
We should consider. ..
Second, pipenv is quite a special case here, as we use the sdist to let people install pipenv . If we add the tests people who wanted to just install and use pipenv will unnecessarily download the tests too.
To fix it, we need to build wheels.
But even if we did that, distributing the tests only would be useless. The reason for this is that pipenv's extended integration tests required many source packages that are included in the repository as git submodules.
Hence, the audience of the sdist file - developers - would come short handed here.
The best way to get the sources with tests is included is to close the repository directly from GitHub.

@oz123
Copy link
Contributor

oz123 commented Dec 11, 2024

Sorry we are building wheels and uploading them to pypi.
I oversaw it when replying from a mobile device.
@matteius I believe we can close the issue.

@oz123 oz123 removed the Contributor Candidate The issue has been identified/triaged and contributions are welcomed/encouraged. label Dec 11, 2024
@mtelka
Copy link
Author

mtelka commented Dec 11, 2024

Hence, the audience of the sdist file - developers - would come short handed here.

In addition to developers the sdist audience are also downstream distributions. Which is my case too. I use sdists to create OpenIndiana packages.

The best way to get the sources with tests is included is to close the repository directly from GitHub.

Yes, I can use github tarballs for that (I actually do so now for pipenv), but github tarballs are unstable (they could change later) and so sdists are always the preferred way for getting the sources.

Thank you.

@oz123
Copy link
Contributor

oz123 commented Dec 11, 2024

@mtelka are you running the complete integration test as part of the build?

The gentoo ebuild is only running the unittest because the gentoo build system does not allow network calles.

@mtelka
Copy link
Author

mtelka commented Dec 11, 2024

@mtelka are you running the complete integration test as part of the build?

I do not know. I just run pytest :-). However many tests are failing and the testing takes about one hour:

===== 54 failed, 325 passed, 18 skipped, 27 warnings in 3919.83s (1:05:19) =====

Unfortunately, I had no time yet to analyze what exactly is failing there.

@oz123
Copy link
Contributor

oz123 commented Dec 11, 2024

I recommend that you just do:

pytest -m "not cli and not needs_internet" tests/unit/

These tests do not detpend on git submodules and do not require a local pypiserver to run.
Also, they should take around no more than 20 seconds on a normal computer.

@mtelka
Copy link
Author

mtelka commented Dec 12, 2024

I recommend that you just do:

pytest -m "not cli and not needs_internet" tests/unit/

Thanks for the hint. It looks far better now:

=========== 143 passed, 8 skipped, 12 deselected, 1 warning in 3.89s ===========

@oz123
Copy link
Contributor

oz123 commented Dec 12, 2024

I guess I can close the ticket?

@mtelka
Copy link
Author

mtelka commented Dec 12, 2024

The sdist is still missing tests :-).

@oz123
Copy link
Contributor

oz123 commented Dec 12, 2024

Well, as I explained, putting the tests there is pointless.
Maybe we can just put the unittests ...

@mtelka
Copy link
Author

mtelka commented Dec 12, 2024

Well, as I explained, putting the tests there is pointless. Maybe we can just put the unittests ...

No objection :-).

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

No branches or pull requests

3 participants