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] Support a way to specify an additional test step between sdist and wheel #856

Open
mgorny opened this issue Feb 1, 2025 · 11 comments
Labels
enhancement New feature or request

Comments

@mgorny
Copy link

mgorny commented Feb 1, 2025

By default, build first builds sdist and then uses it to build wheel — which is a good way to ensure that sdist actually works. However, for projects that also include tests in sdist, it would be awesome to have an option to also run tests on top of sdist, to make sure that they work correctly as well.

What I'm thinking about is a new option that specifies a command to call on top of sdist before wheels are built, and that needs to succeed for the build to progress. Something akin to:

python -m build -t tox

That would:

  1. Build sdist.
  2. Run tox on top of unpacked sdist.
  3. If tox succeeded, build wheels.

I can write a patch if you agree with the idea.

@layday
Copy link
Member

layday commented Feb 1, 2025

Why would you want to run tests against the sdist instead of the installed wheel? Sdists are intermediate artifacts; tests can fail when run against an extracted sdist by design. I'd understand wanting to run tests against an sdist-built wheel during development (instead of a pip-built wheel from source, which is what you get with tox), but what's the benefit otherwise?

@mgorny
Copy link
Author

mgorny commented Feb 1, 2025

Sorry, perhaps I wasn't entirely clear. If people include tests in source distributions, they expect tests to work there. So making sure that tests fail as part of a CI deployment is precisely the point — they should fail for us, so we know that we have made a mistake, rather than for users who run tests once the release is out. Of course, one can implement such a workflow manually, but given that build is perfectly suited for a typical release-upload workflow, including the testing step in there would be convenient.

@layday
Copy link
Member

layday commented Feb 2, 2025

I'm not entirely sure that's a valid assumption to make is what I was getting at. I assume it's gonna work in the majority of cases, but sdists provide no guarantee that the installed packages are gonna look anything like the packages in the sdist. For example, tests might attempt to access an (un)installed console script, or they might need to read the package's dist-info, or they might rely on compiled modules. We'd also have to make some fairly weak assumptions about how the sdist's structured (e.g. that the packages are on the top level) and add the entirety of the sdist root on the Python path, inadvertently exposing non-package folders to Python, which can lead to conflicts - false negatives and false positives.

@mgorny
Copy link
Author

mgorny commented Feb 2, 2025

Well, I'm not saying that build should do anything by default — just give people an option, so they could handle their specific need easier.

@layday
Copy link
Member

layday commented Feb 3, 2025

I'm not in favour of running tests against an sdist because it's fundamentally antithetical to PyPA's package build and distribution model. Tests should always be run against an installable - if not an installed - package. I'm also personally not in favour of providing a testing option in build because it's outside the tool's scope as I imagine it.

If your aim is to run tests in CI, why does that need to happen before installation? Could you not block merging a change request unless the tests pass, for instance? If the aim's to avoid installing broken packages on user machines, I think you'd have a higher chance of success if you were to run the tests against the installed package in a temporary env with the package's dependencies exposed from the outer env. This could go something like this:

  1. Build wheel with build
  2. Create virtual environment with python -m venv --system-site-packages --without-pip <venv-path>
  3. Install the built wheel with installer by passing the venv path to --prefix
  4. Run your tests
  5. If the tests pass, repeat step 3 with your default prefix

Maybe there's more facets to this I'm not understanding - if you could explain the problem you're having in a little more detail, we could see if another alternative would work.

@mgorny
Copy link
Author

mgorny commented Feb 3, 2025

The goal is to provide a working test suite in source distribution, so that people using source distribution (such as downstreams) can successfully run tests. Therefore, it is important to make sure that the source distribution actually includes all files needed to run tests — and the simplest way of doing that is to run tests using the source distribution.

I'm not saying tests should use package sources itself from the source distribution — I'm talking of using tests from the source distribution. The way I see it, a convenient way of doing that would be to run:

python -m build -t tox

which would mean:

  1. build builds and unpacks the source distribution.
  2. tox builds and installs the wheel from it, then runs the suite against the installed package using test sources in the source distribution.
  3. build builds the wheel.

Admittedly, the wheel gets built twice here but that's a minor problem — it happens anyway with the current workflows where tox and build are used separately.

@layday
Copy link
Member

layday commented Feb 3, 2025

I think this is backwards. There's no good reason to let tox build a wheel for you you won't be distributing, besides the fact that it's wasteful. Firstly, tox uses pip as a wheel builder; pip does not use build. tox might be configured to install an editable wheel and pip might call different hooks in a different sequence. Secondly, wheel builds are typically not deterministic. The wheel you'll be testing will differ, even if it's only in the timestamps of its files. You are just adding entropy.

Admittedly, the wheel gets built twice here but that's a minor problem — it happens anyway with the current workflows where tox and build are used separately.

It does, but it'd be better if it didn't. A much better workflow would be:

  1. Build wheel with build
  2. Invoke tox, passing the built wheel as an argument
  3. Job's a good'un

@FFY00
Copy link
Member

FFY00 commented Feb 4, 2025

I think as-of now, this is a non-starter since test running is not standardized. Also, I think this is out of scope for the main CLI, though if test running gets standardized, I am open to maybe provide such as functionality in a build.util CLI, or similar.

@FFY00 FFY00 added the enhancement New feature or request label Feb 4, 2025
@FFY00
Copy link
Member

FFY00 commented Feb 4, 2025

For context, a reason why such a functionality could be desired is downstream packaging, where packaging Python projects could be fully automated.

Eg. In Arch Linux we currently have to manually write instructions for each package: https://gitlab.archlinux.org/archlinux/packaging/packages/rstcheck/-/blob/35395c175954852074a02cd7426144ac83a9c2e5/PKGBUILD#L34-L39

@henryiii
Copy link
Contributor

henryiii commented Feb 4, 2025

I think providing a way to communicate the built wheel names would help this without expanding build's (narrow) scope. It's just a bit tricky to do, since a build can produce stdout and stderr, limiting the options for where we write out the build info (unless using the API instead of CLI, where you can pass data around much easier). uv does this by writing to a file now, originally it was by appending to output, but that was too fragile.

@FFY00
Copy link
Member

FFY00 commented Feb 4, 2025

I think that introduces a fair bit of complexity, at which point I think the better way to deal with this use-case is to write a small script that does exactly what is desired, which is something we make easy to do.

The main use-case for that would be script automation anyway. This is a use-case where wildcards are generally available, so reading the name from some output to use it in another command will probably even be more complex than just using wildcards (eg. dist/*.whl).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants