-
Notifications
You must be signed in to change notification settings - Fork 0
Simplify tests/helpers/maturin.py: inline pin checks, consolidate wheel snapshot (#97, #99) #240
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
base: main
Are you sure you want to change the base?
Changes from all commits
6494aa3
724c455
8b3df4a
cc49f1f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1400,8 +1400,28 @@ change alters the architecture of the lint gate, update | |
|
|
||
| ## Maturin pin synchronization and native wheel tests | ||
|
|
||
| The `tests/helpers/maturin.py` module provides shared helpers for tests that | ||
| validate the maturin version pin contract and native wheel build output. | ||
| These checks live in `cuprum/unittests/test_maturin_build.py`, split across two | ||
| boundaries. The **pin-synchronization** checks are inlined there as private | ||
| helpers (`_read_maturin_pins`, `_read_expected_maturin_version`, | ||
| `_read_manylinux_aarch64_container_ref`, and their regexes): they read | ||
| repository files, have a single consumer, and gain nothing from indirection. | ||
| The **native wheel build and snapshot** machinery stays in | ||
| `tests/helpers/maturin.py`, because it wraps `subprocess` and `zipfile` | ||
| introspection that does not inline cleanly. | ||
|
|
||
| That module's scope is deliberately narrow: build a wheel | ||
| (`build_native_wheel_artifact`), report toolchain availability | ||
| (`toolchain_available`), and normalize a built wheel into a stable snapshot | ||
| (`wheel_build_snapshot`). The build runs `python -m maturin` under the current | ||
| interpreter, so it uses whichever maturin that environment provides rather than | ||
| selecting a version itself. Two separate mechanisms enforce alignment with the | ||
| declared pin: `test_installed_maturin_matches_expected_pin` compares the | ||
| installed CLI against the `pyproject.toml` pin, and the snapshot test asserts | ||
| the built wheel's `Generator` matches that same pin, so a wheel built by an | ||
|
Comment on lines
+1417
to
+1420
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Describe the installed-version check accurately. Replace “compares the installed CLI” with the current-interpreter package Triage: 🤖 Prompt for AI Agents |
||
| unexpected maturin fails the suite. Its re-use policy is to stay that way — do not | ||
| re-externalize further helpers into it until a second concrete consumer exists | ||
| and the shared interface can be designed against real requirements rather than | ||
| anticipated ones. | ||
|
|
||
| **Pin synchronization** (`test_maturin_pins_are_synchronized`) Asserts that the | ||
| maturin version declared in `pyproject.toml`, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the regex ownership description below.
Replace “shared regex” in Lines 1446-1450 with wording that identifies the
private test-local regexes; Lines 1403-1406 now explicitly document that the
helpers and regexes are inlined in
test_maturin_build.py.Triage:
[type:docstyle]🤖 Prompt for AI Agents
Source: Coding guidelines