From bde94061f555d42cca1ed04aaeca3ec685976630 Mon Sep 17 00:00:00 2001 From: lefty Date: Sun, 5 Apr 2026 18:28:51 +0200 Subject: [PATCH 1/3] fix(call): fix check_call! macro to use passed args, add coverage tests The check_call! macros in test_call and test_call_positional ignored their \ parameter, always calling with the outer args.clone() (Bound<'py, PyTuple>). All five invocations therefore tested only the Bound path, leaving &Bound, Py, &Py, and Borrowed impls uncovered. Fix both macros to use \ and \ so each invocation exercises the intended PyCallArgs impl. Add three new tests: - test_call_unit_args: covers ()::call and ()::call_positional impls - test_call_method_positional_default: covers the default call_method_positional trait method (happy path + AttributeError path) - test_call_error_paths: covers the error return paths in Borrowed::call and Borrowed::call_positional (TypeError from PyObject_Call) --- src/call.rs | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 86 insertions(+), 2 deletions(-) diff --git a/src/call.rs b/src/call.rs index b621e83a9e1..33488743a7a 100644 --- a/src/call.rs +++ b/src/call.rs @@ -246,7 +246,7 @@ mod tests { macro_rules! check_call { ($args:expr, $kwargs:expr) => { let (a, k): (Py, Py) = f - .call(args.clone(), Some(kwargs)) + .call($args, Some($kwargs)) .unwrap() .extract() .unwrap(); @@ -287,7 +287,7 @@ mod tests { macro_rules! check_call { ($args:expr, $kwargs:expr) => { let (a, k): (Py, Py) = - f.call1(args.clone()).unwrap().extract().unwrap(); + f.call1($args).unwrap().extract().unwrap(); assert!(a.is(&args)); assert!(k.is_none(py)); }; @@ -309,4 +309,88 @@ mod tests { check_call!(args.as_borrowed(), kwargs); }) } + + #[test] + fn test_call_unit_args() { + // Exercises ()::call and ()::call_positional — both routes through + // into_pyobject_or_pyerr to produce an empty PyTuple. + use crate::{ + types::{IntoPyDict, PyAnyMethods, PyTupleMethods}, + wrap_pyfunction, Py, Python, + }; + use crate::types::PyDict; + + Python::attach(|py| { + let f = wrap_pyfunction!(args_kwargs, py).unwrap(); + let kwargs = &[("x", 1i32)].into_py_dict(py).unwrap(); + + // ()::call — empty positional args dispatched via args.call(…) branch + let (a, _k): (Py, Option>) = + f.call((), Some(kwargs)).unwrap().extract().unwrap(); + assert_eq!(a.bind(py).len(), 0); + + // ()::call_positional — empty positional args dispatched via call1 → args.call_positional(…) + let (a, _k): (Py, Option>) = + f.call1(()).unwrap().extract().unwrap(); + assert_eq!(a.bind(py).len(), 0); + }) + } + + #[test] + fn test_call_method_positional_default() { + // Exercises the default call_method_positional impl on the PyCallArgs trait + // (lines 68–76 of call.rs): getattr + call1. Reached via call_method1. + use crate::{ + exceptions::PyAttributeError, + types::{PyAnyMethods, PyList, PyListMethods, PyTuple}, + Python, + }; + + Python::attach(|py| { + let list = PyList::new(py, [1i32, 2, 3]).unwrap(); + let args = PyTuple::new(py, [4i32]).unwrap(); + + // Happy path: getattr("append") succeeds, call1 appends the element. + list.call_method1("append", args).unwrap(); + assert_eq!(list.len(), 4); + // Verify the value is the integer 4, not a wrapped tuple. + assert_eq!(list.get_item(3).unwrap().extract::().unwrap(), 4i32); + + // Error path: getattr fails → call_method_positional returns Err. + let err = list + .call_method1("nonexistent_method_xyz", PyTuple::empty(py)) + .unwrap_err(); + assert!(err.is_instance_of::(py)); + }) + } + + #[test] + fn test_call_error_paths() { + // Exercises the NULL-return error paths in Borrowed::call and + // Borrowed::call_positional when PyObject_Call fails. + use crate::{ + exceptions::PyTypeError, + types::{IntoPyDict, PyAnyMethods, PyDict}, + Python, + }; + + Python::attach(|py| { + // A PyDict is not callable — invoking it raises TypeError. + let not_callable = PyDict::new(py).into_any(); + let args = PyTuple::empty(py); + let kwargs = &[("x", 1i32)].into_py_dict(py).unwrap(); + + // Borrowed::call error path: kwargs Some → args.call(…) branch → + // PyObject_Call(function=dict, …) returns NULL. + let err = not_callable + .call(args.clone(), Some(kwargs)) + .unwrap_err(); + assert!(err.is_instance_of::(py)); + + // Borrowed::call_positional error path: kwargs None → args.call_positional(…) branch → + // PyObject_Call(…, kwargs=null_ptr) returns NULL. + let err = not_callable.call(args, None).unwrap_err(); + assert!(err.is_instance_of::(py)); + }) + } } From bb16ca13706f48af37cf09e57f901badf2e8d31c Mon Sep 17 00:00:00 2001 From: lefty Date: Sun, 5 Apr 2026 18:30:10 +0200 Subject: [PATCH 2/3] add newsfragments for PR #5951 --- newsfragments/5951.added.md | 1 + newsfragments/5951.fixed.md | 1 + 2 files changed, 2 insertions(+) create mode 100644 newsfragments/5951.added.md create mode 100644 newsfragments/5951.fixed.md diff --git a/newsfragments/5951.added.md b/newsfragments/5951.added.md new file mode 100644 index 00000000000..3b15d147edc --- /dev/null +++ b/newsfragments/5951.added.md @@ -0,0 +1 @@ +Added tests for `PyCallArgs` impls `()`, `&Bound<'py, PyTuple>`, `Py`, `&Py`, and `Borrowed<'_, 'py, PyTuple>`, and for `call_method_positional` default impl and error paths in `src/call.rs`. \ No newline at end of file diff --git a/newsfragments/5951.fixed.md b/newsfragments/5951.fixed.md new file mode 100644 index 00000000000..43c01ef4b2e --- /dev/null +++ b/newsfragments/5951.fixed.md @@ -0,0 +1 @@ +Fixed `check_call!` test macros in `src/call.rs` that ignored their `$args` parameter, causing all five `PyCallArgs` dispatch arms to test only the `Bound<'py, PyTuple>` path. \ No newline at end of file From 2048b73da149e329a77fde7e463991ddb4d2a148 Mon Sep 17 00:00:00 2001 From: lefty Date: Mon, 6 Apr 2026 07:32:57 +0200 Subject: [PATCH 3/3] fix fmt: rustfmt check_call! chain and use-import ordering in tests --- src/call.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/call.rs b/src/call.rs index 33488743a7a..0fdd63ceee6 100644 --- a/src/call.rs +++ b/src/call.rs @@ -245,11 +245,8 @@ mod tests { macro_rules! check_call { ($args:expr, $kwargs:expr) => { - let (a, k): (Py, Py) = f - .call($args, Some($kwargs)) - .unwrap() - .extract() - .unwrap(); + let (a, k): (Py, Py) = + f.call($args, Some($kwargs)).unwrap().extract().unwrap(); assert!(a.is(&args)); assert!(k.is(kwargs)); }; @@ -314,11 +311,11 @@ mod tests { fn test_call_unit_args() { // Exercises ()::call and ()::call_positional — both routes through // into_pyobject_or_pyerr to produce an empty PyTuple. + use crate::types::PyDict; use crate::{ types::{IntoPyDict, PyAnyMethods, PyTupleMethods}, wrap_pyfunction, Py, Python, }; - use crate::types::PyDict; Python::attach(|py| { let f = wrap_pyfunction!(args_kwargs, py).unwrap(); @@ -382,9 +379,7 @@ mod tests { // Borrowed::call error path: kwargs Some → args.call(…) branch → // PyObject_Call(function=dict, …) returns NULL. - let err = not_callable - .call(args.clone(), Some(kwargs)) - .unwrap_err(); + let err = not_callable.call(args.clone(), Some(kwargs)).unwrap_err(); assert!(err.is_instance_of::(py)); // Borrowed::call_positional error path: kwargs None → args.call_positional(…) branch →