Repository navigation
feat(ci): tests-python.yml for the Python SDK workspace - #1406
Conversation
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
The Python workspace had no CI. The new root workflow runs, on pull requests touching the workspace or the crates it compiles and on every push to main: - `python`, a matrix on Python 3.9 (the requires-python floor) and 3.14: cargo fmt, clippy, `cargo test --locked` for both crates, and pytest, through the workspace's mise tasks. MISE_PYTHON_VERSION moves a leg off the local version, and a step fails the leg if that did not take. - `wheels`, one abi3-py39 release wheel per protect-ffi platform (manylinux 2.28, musllinux 1.2, two Darwin, Windows), each checked by filename, imported on 3.9 where the runner matches, uploaded as an artifact, with its build time in the job summary; `wheels-summary` requires six and writes one table. No job holds a CS_* credential. Swatinem/rust-cache is on both compiling jobs. scripts/__tests__/python-ci.test.mjs holds the workflow to the tree: the path filter to the workspace's Cargo.lock path dependencies plus the EQL codegen and catalog, the Python matrix to the floor and the local version, the wheel matrix to the six platform packages, and every workspace task to a step that runs it. Two things the workflow made necessary: - languages/python/Cargo.lock was stale against the root crates (clww-ore 0.6.0, vitaminc 0.8.0) and every `--locked` check refused it. Re-seeded from the root lock and pruned; no shared crate differs now. - eql-suite-ci.test.mjs read `mise run test` from any workflow as EQL's `test` task. Its scan now counts a `mise run` only from a step whose working directory is under packages/eql, with fixture tests. Also excludes languages/python/** from tests.yml's live gate, records the push/pull_request asymmetry in workflow-paths-filter-parity.test.mjs, and updates AGENTS.md and the Python mise.toml notes. No Python lockfile: the package has no runtime dependency, so only pytest could be locked. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Plain-language pass over the workflow comments, the test comments, and the AGENTS.md bullet: short sentences, no idioms, and the interpreter check step named as a sentence. The timeout comment states a fact and no longer carries an open task. AGENTS.md now says the Python workspace runs plain `cargo test`, not nextest, and why: no test there sets an environment variable, and nextest comes from mise.test.toml, which its CI job does not load. `triggers(wf)` and `runWorkingDirectory(wf, job, step)` move into scripts/__tests__/lib/workflows.mjs. shared-test-helpers.test.mjs rejects a local copy of an exported helper, so the six guards that carried their own `triggers` switch to the shared one in this commit. The wheel count is written once, in python-ci.test.mjs. wheels-summary derives its expected count from its platform list. The clock comment says what the recorded build time includes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The three Linux wheel legs failed on the first run. maturin-action builds them in a manylinux container, as root, on a bind mount the runner owns, and the core crate's build script could not read the commit from git there. The script accepts STACKENCRYPT_ENGINE_COMMIT in place of git, so the host reads `git rev-parse HEAD` and the maturin step sets it. maturin-action forwards only variables on its own prefix list into the container, so `docker-options` names it too. All six legs use the override, so every wheel names its engine the same way. python-ci.test.mjs reads the variable name from build.rs and requires both the env entry and the forwarding on every maturin-action step. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
a607e6a to
ce8f5f9
Compare
Lower the wheels timeout to 30 minutes now that two green runs have measured the legs at 1 to 5 minutes, and say so in the comment. Name tests.yml's `changes` filter, the job that contains the exclusion, instead of a "gate" it does not have. Split the comment sentences over 25 words, replace two idioms, and drop the stale count from the `triggers` header. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
cipherstash-agent
left a comment
cipherstash-agent
left a comment
There was a problem hiding this comment.
Review attestation
Reviewed d0a8e3a · posted from Claude Code · claude-sonnet-5-5 · effort unknown
Sources: code-review · model unknown; manual diff read · claude-sonnet-5-5
Skills: code-review, company-skills:review-cross-check, company-skills:review-post
Findings (3): 0 critical, 0 major, 3 minor — 3 inline, 0 below.
Checked against existing feedback (changeset-bot, CodeRabbit skip notice): none of the three is already raised.
Generated by Claude Code
cipherstash-bot
left a comment
cipherstash-bot
left a comment
There was a problem hiding this comment.
Recommendation: 🟡 merge after changes
Change one thing before merging: the new AGENTS.md text says the Python Cargo.lock is seeded from the root lock, and at this commit it is not. The maturin pin and the four test-coverage gaps can wait for a follow-up.
Other findings not posted as comments
- Optional: the credential test in
scripts/__tests__/python-ci.test.mjs(near line 282) matches onlysecrets.CS_andvars.CS_. The bracket formsecrets['CS_CLIENT_KEY']passes it.
How this review was made
| Agent | Model | Review type | Result |
|---|---|---|---|
| claude | claude-opus-5-5 | test-gap | 4 found, 4 posted |
| claude | claude-opus-5-5 | rust | 0 found, 0 posted |
| claude | claude-opus-5-5 | python | 0 found, 0 posted |
| claude | claude-opus-5-5 | infracode | 2 found, 2 posted |
| codex | gpt-5.6-terra | test-gap | 0 found, 0 posted |
| codex | gpt-5.6-terra | rust | 0 found, 0 posted |
| codex | gpt-5.6-terra | python | 0 found, 0 posted |
| codex | gpt-5.6-terra | infracode | 0 found, 0 posted |
Synthesis: claude-sonnet-5-5 merged the findings, removed duplicates and dropped findings it could not confirm in the code. 0 posted finding(s) were raised by two or more models.
Plain language: the reader test did not run; only the word-and-shape check was applied.
Stack: position 2 of 15 (#1405, *️⃣ #1406, #1414, #1415, #1433, #1437, #1453, #1499, #1509, #1510, #1511, #1513, #1516, #1517, #1582). *️⃣ marks this pull request.
Context loaded: the description, 2 linked issue(s) and 6 discussion entries.
Summary
The Python SDK at
languages/pythonhad no CI. This addstests-python.yml. It runs the workspace's Rust checks and pytest on Python 3.9 and 3.14, and builds one wheel for each of the six platforms the FFI ships for. Nothing is published.Changes
.github/workflows/tests-python.yml: two jobs.pythonruns fmt, clippy,cargo test --locked, and pytest on both interpreters.wheelsbuilds six abi3-py39 release wheels, uploads them as artifacts, and writes the build time per platform to the job summary. No job receives aCS_*credential.scripts/__tests__/python-ci.test.mjs: holds the workflow to the tree. The path filter must cover every path dependency in the workspace'sCargo.lock. The Python matrix must be therequires-pythonfloor and the version inmise.toml. The wheel matrix must match the protect-ffi platform packages.scripts/__tests__/eql-suite-ci.test.mjs: counts amise runas an EQL task only from a step that runs underpackages/eql. Before this,mise run testfromlanguages/pythonwas read as EQL'stesttask.scripts/__tests__/lib/workflows.mjs: two shared helpers,triggersandrunWorkingDirectory. Six existing guards switch totriggers.languages/python/Cargo.lock: re-seeded from the root lock. The old one named crate versions the root no longer uses, so every--lockedcheck failed.tests.yml: excludeslanguages/python/**from its live gate, as it does for the Go module.AGENTS.md,languages/python/mise.toml: notes on the workflow, the lock re-seeding, and why this workspace uses plaincargo test.Verification
scripts/__tests__meta-tests: 83 files pass.languages/python:cargo fmt --all --check,cargo clippy --locked,mise run test:cargo, andmise run testpass locally.run:block.Related
Closes #1383. Stacked on #1405. The live test job is #1389.
Review notes
This PR adds no Python lockfile. A
uv.lockcomes later, in a new PR at the top of this stack.Today CI resolves every Python package from PyPI on each run. The packages are maturin and the
testextra. By the end of the stack, that extra lists pytest, attrs, Pydantic, SQLAlchemy, and psycopg. The package itself has no runtime dependency, so a lock affects only CI and local builds.The lock waits for the end of the stack for two reasons:
Until the lock lands, three risks apply:
livejob installs releases nobody has checked, while it has theCS_*secrets in its environment.🤖 Generated with Claude Code