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.

Sign up for GitHub

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

Jump to bottom

fix: correctly exclude tests from package #384

Merged
merged 2 commits into from
Apr 27, 2022
Merged

fix: correctly exclude tests from package #384

merged 2 commits into from
Apr 27, 2022

Conversation

ghickman
Copy link
Contributor

@ghickman ghickman commented Apr 26, 2022 •
edited
Loading

This improves our packaging story by doing a number of things:

  • test the installed package via one of its entrypoints to avoid python -m using the code on-disk
  • exclude tests and its sub modules
  • check the build distribution doesn't include tests by interrogating the built wheel
  • pull the building and testing of distributions into the justfile for easier local testing

@ghickman
fix: revert to excluding tests using the exclude kwarg
3691b20 
Target the tests _modules_ using "tests*" so we capture its sub modules
as well.
@ghickman ghickman changed the title fix: explicitly set the directory to look for packages in fix: correctly exclude tests from package Apr 27, 2022
@ghickman ghickman marked this pull request as ready for review April 27, 2022 13:09
@ghickman ghickman merged commit b8dae48 into master Apr 27, 2022
@ghickman ghickman deleted the package-sub-dirs branch April 27, 2022 14:31
madwort
madwort reviewed Aug 30, 2023
View reviewed changes
justfile
$VENV/bin/local_run --help

# check we haven't packaged tests with it
unzip -Z -1 dist/*.whl | grep -vq "^tests/"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in #626

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@madwort madwort madwort left review comments

@bloodearnest bloodearnest bloodearnest approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ghickman @bloodearnest @madwort