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 #1674

Closed
1 of 2 tasks
mtelka opened this issue Jun 20, 2023 · 8 comments
Closed
1 of 2 tasks

sdist is missing tests #1674

mtelka opened this issue Jun 20, 2023 · 8 comments
Labels
bug Something isn't working

Comments

@mtelka
Copy link

mtelka commented Jun 20, 2023

Bug Description

The sdist package as PyPI is missing tests, although noxfile.py file is in the sdist. Please add tests to sdist to make downstream testing easier. Thank you.

Your maturin version (maturin --version)

1.1.0

Your Python version (python -V)

N/A

Your pip version (pip -V)

N/A

What bindings you're using

None

Does cargo build work?

  • Yes, it works

If on windows, have you checked that you aren't accidentally using unix path (those with the forward slash /)?

  • Yes

Steps to Reproduce

  1. Dowload sdist from PyPI and unpack it.
@mtelka mtelka added the bug Something isn't working label Jun 20, 2023
@messense
Copy link
Member

tests are written in Rust, what downstream would run Rust tests for a Python sdist?

@mtelka
Copy link
Author

mtelka commented Jun 20, 2023

I'm packaging maturin for OpenIndiana and testing is something that could find incompatibilities either in the operating system or in the Python project. This is especially valuable for not so common platforms like OpenIndiana.

@messense
Copy link
Member

Thanks, I understand the benefits. I'm just not sure whether adding tests to sdist would work since it needs to be run by cargo test.

And why not using the github release .tar.gz source, for example https://github.com/PyO3/maturin/archive/refs/tags/v1.1.0.tar.gz? It contains all of the code.

@mtelka
Copy link
Author

mtelka commented Jun 20, 2023

Github tarball is possible too, but it creates a bit more maintenance work because there is no simple way how to find the tarball automatically. The sdist url is directly referenced from PyPI so it could be easily found without manual intervention.

BTW, if tests should be run using cargo test why there is noxfile.py file?

@messense
Copy link
Member

noxfile.py is only for automating some Emscripten stuff.

@konstin
Copy link
Member

konstin commented Jun 20, 2023

The sdist url is directly referenced from PyPI so it could be easily found without manual intervention.

fwiw the github api provides a way to get the tarball of the latest release, and we also link the github url from pypi, so you might be able to get your automation this way: https://docs.github.com/de/rest/releases/releases?apiVersion=2022-11-28#get-the-latest-release

@mgorny
Copy link

mgorny commented Jun 24, 2023

And why not using the github release .tar.gz source, for example https://github.com/PyO3/maturin/archive/refs/tags/v1.1.0.tar.gz? It contains all of the code.

Git doesn't guarantee that the generated archives will be stable. There have already been cases in the past when changes in git or one of its dependencies suddenly caused all archives to change and broke all downstream consumers via checksum mismatches.

@messense
Copy link
Member

I'm going to close this because we don't have any tests written in Python to distribute, and I'm removing noxfile.py from sdist in #1679.

@messense messense closed this as not planned Won't fix, can't repro, duplicate, stale Jun 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants