Skip to content

Run tests with macros and without experimental inspect #5913

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

Open
Tpt wants to merge 1 commit into
base: main
Choose a base branch
from
+5 −33
Open

Run tests with macros and without experimental inspect #5913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion noxfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1494,7 +1494,7 @@ def _get_feature_sets() -> Tuple[Optional[str], ...]:
if is_rust_nightly():
features += ",nightly"

return (None, "abi3", features, f"abi3,{features}")
return (None, "abi3", "macros", "abi3,macros", features, f"abi3,{features}")
Copy link
Copy Markdown
Contributor

@ngoldbaum ngoldbaum Mar 26, 2026

Choose a reason for hiding this comment

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

There's a trade-off that doing this will make CI more expensive. @davidhewitt do you happen to have context for why you excluded testing just the "macros" feature here?

Copy link
Copy Markdown
Member

@davidhewitt davidhewitt Mar 31, 2026

Choose a reason for hiding this comment

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

I think the assumption is that the full feature set is purely additive on top of the base "macros" feature.

Strictly speaking, that's true for everywhere except two cases:

  • The UI tests emit different output depending on the types involved
  • The multiple-pymethods feature replaces some of the default functionality.

Looks like this adds ~2 minutes to each CI job. Comparing ubuntu x64 stable jobs:

This PR:

Thu, 26 Mar 2026 09:02:08 GMT Run nox -s test-rust
Thu, 26 Mar 2026 09:02:08 GMT nox > Running session test-rust
Thu, 26 Mar 2026 09:02:08 GMT nox > cargo test --no-fail-fast --package=pyo3-build-config
Thu, 26 Mar 2026 09:02:09 GMT nox > cargo test --no-fail-fast --package=pyo3-macros-backend
Thu, 26 Mar 2026 09:02:15 GMT nox > cargo test --no-fail-fast --package=pyo3-macros
Thu, 26 Mar 2026 09:02:20 GMT nox > cargo test --no-fail-fast --package=pyo3-ffi
Thu, 26 Mar 2026 09:02:22 GMT nox > cargo test --no-fail-fast --no-default-features --all-targets
Thu, 26 Mar 2026 09:02:47 GMT nox > cargo test --no-fail-fast --features=abi3 --no-default-features --all-targets
Thu, 26 Mar 2026 09:02:58 GMT nox > cargo test --no-fail-fast --features=macros --no-default-features --all-targets
Thu, 26 Mar 2026 09:04:06 GMT nox > cargo test --no-fail-fast --features=abi3,macros --no-default-features --all-targets
Thu, 26 Mar 2026 09:04:42 GMT nox > cargo test --no-fail-fast --features=full,multiple-pymethods --no-default-features
Thu, 26 Mar 2026 09:06:30 GMT nox > cargo test --no-fail-fast --features=abi3,full,multiple-pymethods --no-default-features
Thu, 26 Mar 2026 09:07:50 GMT nox > cargo test --no-fail-fast --features=abi3-py37,full,multiple-pymethods --no-default-features
Thu, 26 Mar 2026 09:09:09 GMT nox > Session test-rust was successful in 7 minutes.

Recent main:

Tue, 31 Mar 2026 09:30:38 GMT Run nox -s test-rust
Tue, 31 Mar 2026 09:30:38 GMT nox > Running session test-rust
Tue, 31 Mar 2026 09:30:38 GMT nox > cargo test --no-fail-fast --package=pyo3-build-config
Tue, 31 Mar 2026 09:30:39 GMT nox > cargo test --no-fail-fast --package=pyo3-macros-backend
Tue, 31 Mar 2026 09:30:45 GMT nox > cargo test --no-fail-fast --package=pyo3-macros
Tue, 31 Mar 2026 09:30:50 GMT nox > cargo test --no-fail-fast --package=pyo3-ffi
Tue, 31 Mar 2026 09:30:51 GMT nox > cargo test --no-fail-fast --no-default-features --all-targets
Tue, 31 Mar 2026 09:31:16 GMT nox > cargo test --no-fail-fast --features=abi3 --no-default-features --all-targets
Tue, 31 Mar 2026 09:31:26 GMT nox > cargo test --no-fail-fast --features=full,multiple-pymethods --no-default-features
Tue, 31 Mar 2026 09:33:31 GMT nox > cargo test --no-fail-fast --features=abi3,full,multiple-pymethods --no-default-features
Tue, 31 Mar 2026 09:34:51 GMT nox > cargo test --no-fail-fast --features=abi3-py38,full,multiple-pymethods --no-default-features
Tue, 31 Mar 2026 09:36:08 GMT nox > Session test-rust was successful in 5 minutes.

I am unsure what the right call is here. I am generally pro-coverage however the value add diminishes with each corner case and the CI cost here is quite high.

I have #5862 and #5863 which both should provide positive impacts to CI performance, maybe we can re-evaluate what to do here once those have landed?

Copy link
Copy Markdown
Contributor

@ngoldbaum ngoldbaum Mar 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe for now we split off the fixes in this PR into a new PR separate from the CI changes?

Copy link
Copy Markdown
Contributor Author

@Tpt Tpt Mar 31, 2026

Choose a reason for hiding this comment

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

Done #5931



_RELEASE_LINE_START = "release: "
Expand Down
3 changes: 2 additions & 1 deletion tests/test_compile_error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ fn test_compile_errors() {
#[cfg(not(feature = "experimental-inspect"))]
t.compile_fail("tests/ui/invalid_property_args.rs");
t.compile_fail("tests/ui/invalid_proto_pymethods.rs");
#[cfg(not(feature = "experimental-inspect"))]
#[cfg(not(all(Py_LIMITED_API, not(Py_3_10))))] // to avoid PyFunctionArgument for &str
t.compile_fail("tests/ui/invalid_pyclass_args.rs");
t.compile_fail("tests/ui/invalid_pyclass_doc.rs");
Expand Down Expand Up @@ -78,7 +79,7 @@ fn test_compile_errors() {
t.pass("tests/ui/pymodule_missing_docs.rs");
#[cfg(not(any(Py_LIMITED_API, feature = "experimental-inspect")))]
t.pass("tests/ui/forbid_unsafe.rs");
#[cfg(all(Py_LIMITED_API, not(feature = "experimental-async")))]
#[cfg(all(Py_LIMITED_API, not(Py_3_12), not(feature = "experimental-async")))]
// output changes with async feature
t.compile_fail("tests/ui/abi3_inheritance.rs");
#[cfg(all(Py_LIMITED_API, not(Py_3_9)))]
Expand Down
33 changes: 2 additions & 31 deletions tests/ui/invalid_pyclass_args.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,35 +192,6 @@ error[E0119]: conflicting implementations of trait `PyClassNewTextSignature` for
|
= note: this error originates in the attribute macro `pymethods` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: `Box<dyn std::error::Error + Send + Sync>` cannot be used as a Python function argument
--> tests/ui/invalid_pyclass_args.rs:205:12
|
205 | field: Box<dyn std::error::Error + Send + Sync>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `PyFunctionArgument<'_, '_, '_, false>` is not implemented for `Box<dyn std::error::Error + Send + Sync>`
|
= note: implement `FromPyObject` to enable using `Box<dyn std::error::Error + Send + Sync>` as a function argument
= note: `Python<'py>` is also a valid argument type to pass the Python token into `#[pyfunction]`s and `#[pymethods]`
help: the following other types implement trait `PyFunctionArgument<'a, 'holder, 'py, IMPLEMENTS_FROMPYOBJECT>`
--> src/impl_/extract_argument.rs
|
| / impl<'a, 'holder, 'py, T: 'a + 'py> PyFunctionArgument<'a, 'holder, 'py, false>
| | for &'holder Bound<'py, T>
| | where
| | T: PyTypeCheck,
| |___________________^ `&'holder pyo3::Bound<'py, T>` implements `PyFunctionArgument<'a, 'holder, 'py, false>`
...
| / impl<'a, 'holder, 'py, T> PyFunctionArgument<'a, 'holder, 'py, false> for Option<T>
| | where
| | T: PyFunctionArgument<'a, 'holder, 'py, false>,
| |___________________________________________________^ `Option<T>` implements `PyFunctionArgument<'a, 'holder, 'py, false>`
...
| impl<'a, 'holder, T: PyClass> PyFunctionArgument<'a, 'holder, '_, false> for &'holder T {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `&'holder T` implements `PyFunctionArgument<'a, 'holder, '_, false>`
...
| / impl<'a, 'holder, T: PyClass<Frozen = False>> PyFunctionArgument<'a, 'holder, '_, false>
| | for &'holder mut T
| |______________________^ `&'holder mut T` implements `PyFunctionArgument<'a, 'holder, '_, false>`

error[E0592]: duplicate definitions with name `__pymethod___richcmp____`
--> tests/ui/invalid_pyclass_args.rs:37:1
|
Expand Down Expand Up @@ -456,11 +427,11 @@ error[E0277]: `Box<dyn std::error::Error + Send + Sync>` cannot be used as a Pyt
--> tests/ui/invalid_pyclass_args.rs:205:12
|
205 | field: Box<dyn std::error::Error + Send + Sync>,
| ^^^ the trait `pyo3::PyClass` is not implemented for `Box<dyn std::error::Error + Send + Sync>`
| ^^^ the trait `PyClass` is not implemented for `Box<dyn std::error::Error + Send + Sync>`
|
= note: implement `FromPyObject` to enable using `Box<dyn std::error::Error + Send + Sync>` as a function argument
= note: `Python<'py>` is also a valid argument type to pass the Python token into `#[pyfunction]`s and `#[pymethods]`
= help: the following other types implement trait `pyo3::PyClass`:
= help: the following other types implement trait `PyClass`:
Coord
Coord2
Coord3
Expand Down
Loading