Ship the library in the sdist, and stop shipping the tests - #93
Merged
Merged
Conversation
tests/ was in the sdist include list from the commit that introduced packaging, with no stated reason, and against the wheel comment in the same file: mqtt_demo/, tests/ and setup_cert.py are in-repo development material. Nothing had ever run them from an sdist, because the packages they import stayed behind. Two files failed at collection on tools/, five tests failed reading docs/, and seven more failed at collection on mqtt_demo and setup_cert. Shipping those directories to satisfy the tests was the wrong direction: it let test placement decide what the package distributes, and the five docs tests check documentation hygiene rather than anything a packager cares about. The sdist now carries the library and its metadata, which is what the file already said it intended, and check_distribution.py enforces that set. The sdist halves, 213501 bytes to 107177. Nothing else changes: the wheel was already library-only, the repo suite still runs 798, CI's sdist job installs the artefact and imports from it as before, and anyone wanting the suite uses the git tag, as CI does.
QuiteYellow
force-pushed
the
fix/ship-tools-with-the-tests
branch
from
September 13, 2026 11:26
9cd2c02 to
b8133ed
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tests/has been in the sdist include list since the commit that introduced packaging, with no stated reason, and against the wheel comment in the same file:mqtt_demo/,tests/andsetup_cert.pyare in-repo development material.Nothing had ever run them from an sdist, because the packages they import stayed behind. From a bare extraction:
tools/;docs/;mqtt_demoorsetup_cert.CI never noticed, because its sdist job installs the artefact and imports one module.
Why this does the opposite of shipping them
The first version of this branch shipped
tools/anddocs/so the tests would run. That let test placement decide what the package distributes, which is backwards. It also put documentation-hygiene checks in a packager's path: whetherdocs/api.mdmatches the code, and whether this repository's markdown links resolve, has no bearing on a build of the library.So the sdist now carries the library and its metadata, which is what the file already said it intended.
check_distribution.pyenforces that set.What this costs
Nothing identified. The wheel was already library-only. The repository suite still runs 798. CI's sdist job installs the artefact and imports from it exactly as before, verified against the new one. Anyone wanting to run the suite uses the git tag, as CI does.
The sdist halves, 213501 bytes to 107177.
Validation
798 tests pass on Python 3.11, 3.13 and 3.14, and on the dependency floor.
check_distribution.pypasses on both artefacts, andcheck_share_safety.pyis clean.