From 021c78655e1abb90433327a423a6b7570536ecec Mon Sep 17 00:00:00 2001 From: James Clarke Date: Mon, 25 May 2026 14:02:22 +0100 Subject: [PATCH] docs: expand AGENTS.md with test marker guidance and CI audit notes (#7963) * docs: expand AGENTS.md with test marker guidance and CI audit notes Document the four-way test marker decision (expectedFailure / expectedFailureIf vs skip / skipIf, with skip reserved for tests that crash the interpreter), add guidance on avoiding zizmor's impostor-commit audit when pinning third-party actions, link the wiki guide for syncing tests from upstream CPython, and note the grep recipe for finding WIP TODO: RUSTPYTHON entries. Co-Authored-By: Claude Opus 4.7 (1M context) * docs: add pre-commit checks rule, trim CI section per review Add a "CRITICAL: Pre-commit Checks" subsection requiring prek and the full test suite to pass before any commit. Shrink the CI Workflows section to a single sentence per @ShaharNaveh's review feedback: contributors only need to know that workflow changes must pass a zizmor scan in CI, not the full impostor-commit explainer. Co-Authored-By: Claude Opus 4.7 (1M context) * docs: warn against doc comments on pyattr/pyclass/pyfunction items - Note that rustpython-doc supplies CPython docstrings via the derive macros - Flag that `///` comments override that source and are dropped on `#[pyattr]` --------- Co-authored-by: Claude Opus 4.7 (1M context) --- AGENTS.md | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index b407328cf..c89b2a4d3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -38,6 +38,12 @@ RustPython is a Python 3 interpreter written in Rust, implementing Python 3.14.0 - Always ask the user before performing any git operations that affect the remote repository - Commits can be created locally when requested, but pushing and PR creation require explicit approval +**CRITICAL: Pre-commit Checks** +- Before creating ANY commit, you MUST run `prek run --all-files` (or `pre-commit run --all-files`) AND the full test suite. Both must pass — do not commit if either fails. +- Test commands are documented in the [Testing](#testing) section below. At minimum run `cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher`; if the change touches `extra_tests/snippets/` run `pytest -v` there too, and if it touches `Lib/` or interpreter behavior, run the relevant `cargo run --release -- -m test ` modules. +- If a hook auto-fixes files (e.g. `ruff-format`, `rustfmt`), re-stage the fixes, re-run `prek` until it reports a clean pass, then re-run the tests, then commit. +- NEVER bypass these checks with `--no-verify`, `--no-gpg-sign`, or by skipping tests "because the change is small". If a hook or test fails, fix the underlying issue and create a new commit — do not amend or force the failing commit through. + ## Important Development Notes ### Running Python Code @@ -81,6 +87,35 @@ The `Lib/` directory contains Python standard library files copied from the CPyt - `unittest.skip("TODO: RustPython ")` - `unittest.expectedFailure` with `# TODO: RUSTPYTHON ` comment +#### Choosing the right marker + +When marking a test that fails on RustPython, prefer one of the following forms: + +```python +@unittest.expectedFailure # TODO: RUSTPYTHON; +# or +@unittest.expectedFailureIf(, "TODO: RUSTPYTHON; ") +``` + +If the test would crash the interpreter (segfault, Rust panic, abort, infinite loop), use `skip` instead so the rest of the suite can still run: + +```python +@unittest.skip("TODO: RUSTPYTHON; ") +# or +@unittest.skipIf(, "TODO: RUSTPYTHON; ") +``` + +**When to use which:** + +- **Prefer `expectedFailure` / `expectedFailureIf`** by default. The test body still runs, so if RustPython is later fixed, the unexpected pass surfaces immediately and the decorator can be removed. Use the conditional `*If` form when the failure is environment-specific (e.g., a platform or build flag). +- **Use `skip` / `skipIf` only when running the test would take down the test process** — segfaults, Rust panics, aborts, or hangs that block subsequent tests. Skipping keeps the suite usable; `expectedFailure` cannot help here, because the test body still executes. + +To find WIP entries that are partly modified and may need follow-up: + +```bash +grep -d recurse 'TODO: RUSTPYTHON' Lib/test/ +``` + ### Clean Build When you modify bytecode instructions, a full clean is required: @@ -129,6 +164,7 @@ Run `./scripts/whats_left.py` to get a list of unimplemented methods, which is h - Do not delete or rewrite existing comments unless they are factually wrong or directly contradict the new code. - Do not add decorative section separators (e.g. `// -----------`, `// ===`, `/* *** */`). Use `///` doc-comments or short `//` comments only when they add value. +- Do not put `///` doc comments on items annotated with `#[pyattr]`, `#[pyclass]`, or `#[pyfunction]`. The derive macros pull authoritative docstrings from CPython via the `rustpython-doc` crate; a Rust doc comment overrides that source, and on `#[pyattr]` it is silently dropped. #### Avoid Duplicate Code in Branches @@ -258,9 +294,14 @@ See DEVELOPMENT.md "CPython Version Upgrade Checklist" section. - Document that it requires PEP 695 support - Focus on tests that can be fixed through Rust code changes only +## CI Workflows + +If you modify any file under `.github/workflows/`, the change must pass a [zizmor](https://docs.zizmor.sh/) scan in CI. + ## Documentation - Check the [architecture document](/architecture/architecture.md) for a high-level overview - Read the [development guide](/DEVELOPMENT.md) for detailed setup instructions - Generate documentation with `cargo doc --no-deps --all` - Online documentation is available at [docs.rs/rustpython](https://docs.rs/rustpython/) +- [How to update test files](https://github.com/RustPython/RustPython/wiki/How-to-update-test-files#checkout-cpython-source-code-initial-setup) — guide for syncing test cases from upstream CPython into the `Lib/` directory