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

"Embed" test_bdist_wheel files and execution #4429

Merged
merged 5 commits into from
Jun 25, 2024

Conversation

abravalheri
Copy link
Contributor

@abravalheri abravalheri commented Jun 20, 2024

Summary of changes

Currently test_bdist_wheel:

  • Relies on external files placed on the setuptools/tests/bdist_wheel_testdata:
    • The problem with this approach is that it complicates the management of MANIFEST.in and is error prone (see [BUG] Test data missing (extension.c files and more) missing in 70.1.0 sdist #4428).
      Some files are missing in the sdist and others may be erroneously added depending on how clean your directory is (e.g. setuptools/tests/bdist_wheel_testdata/*/build).
      This affects setuptools a lot because we cannot use setuptools-scm, but even if we could it also complicates "git" management itself...
      I found a couple of files that should not have been committed (e.g. .so files that were generated by tests).
  • Invokes python setup.py bdist_wheel via subprocess:
    • The problem with this approach (other than using the deprecated CLI of setup.py) is that it complicates collecting coverage data.

This PR "embeds" both of these aspects:

  • all required files are generated during the tests runtime on temporary directories
  • the bdist_wheel command runs via API in the current process.

Closes #4428

Pull Request Checklist

@abravalheri abravalheri marked this pull request as ready for review June 20, 2024 12:06
@abravalheri abravalheri merged commit a66cedc into pypa:main Jun 25, 2024
22 checks passed
@abravalheri abravalheri deleted the issue-4428 branch June 25, 2024 08:42
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 this pull request may close these issues.

[BUG] Test data missing (extension.c files and more) missing in 70.1.0 sdist
1 participant