-
Notifications
You must be signed in to change notification settings - Fork 901
ci: enable more tests on 3.14t #5524
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -95,7 +95,7 @@ fn test_buffer_referenced() { | |
| } | ||
|
|
||
| #[test] | ||
| #[cfg(all(Py_3_8, not(Py_GIL_DISABLED)))] // sys.unraisablehook not available until Python 3.8 | ||
| #[cfg(Py_3_8)] // sys.unraisablehook not available until Python 3.8 | ||
| fn test_releasebuffer_unraisable_error() { | ||
| use pyo3::exceptions::PyValueError; | ||
| use test_utils::UnraisableCapture; | ||
|
|
@@ -120,20 +120,20 @@ fn test_releasebuffer_unraisable_error() { | |
| } | ||
|
|
||
| Python::attach(|py| { | ||
| let capture = UnraisableCapture::install(py); | ||
|
|
||
| let instance = Py::new(py, ReleaseBufferError {}).unwrap(); | ||
| let env = [("ob", instance.clone_ref(py))].into_py_dict(py).unwrap(); | ||
|
|
||
| assert!(capture.borrow(py).capture.is_none()); | ||
| let (err, object) = UnraisableCapture::enter(py, |capture| { | ||
| let env = [("ob", instance.clone_ref(py))].into_py_dict(py).unwrap(); | ||
|
|
||
| assert!(capture.take_capture().is_none()); | ||
|
|
||
| py_assert!(py, *env, "bytes(ob) == b'hello world'"); | ||
| py_assert!(py, *env, "bytes(ob) == b'hello world'"); | ||
|
|
||
| capture.take_capture().unwrap() | ||
| }); | ||
|
|
||
| let (err, object) = capture.borrow_mut(py).capture.take().unwrap(); | ||
| assert_eq!(err.to_string(), "ValueError: oh dear"); | ||
| assert!(object.is(&instance)); | ||
|
|
||
| capture.borrow_mut(py).uninstall(py); | ||
|
Contributor
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. even without the locking the new API is nicer without this explicit uninstall step.
Member
Author
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. Agreed, much less error prone. |
||
| }); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| #![cfg(feature = "macros")] | ||
|
|
||
| #[cfg(Py_3_8)] | ||
| use pyo3::ffi::c_str; | ||
| use pyo3::prelude::*; | ||
| use pyo3::types::PyType; | ||
| use pyo3::{py_run, PyClass}; | ||
|
|
@@ -615,7 +617,7 @@ fn access_frozen_class_without_gil() { | |
| } | ||
|
|
||
| #[test] | ||
| #[cfg(all(Py_3_8, not(Py_GIL_DISABLED)))] // sys.unraisablehook not available until Python 3.8 | ||
| #[cfg(Py_3_8)] | ||
| #[cfg_attr(target_arch = "wasm32", ignore)] | ||
| fn drop_unsendable_elsewhere() { | ||
| use std::sync::{ | ||
|
|
@@ -637,35 +639,40 @@ fn drop_unsendable_elsewhere() { | |
| } | ||
|
|
||
| Python::attach(|py| { | ||
| let capture = UnraisableCapture::install(py); | ||
| let (err, object) = UnraisableCapture::enter(py, |capture| { | ||
| let dropped = Arc::new(AtomicBool::new(false)); | ||
|
|
||
| let dropped = Arc::new(AtomicBool::new(false)); | ||
|
|
||
| let unsendable = Py::new( | ||
| py, | ||
| Unsendable { | ||
| dropped: dropped.clone(), | ||
| }, | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| py.detach(|| { | ||
| spawn(move || { | ||
| Python::attach(move |_py| { | ||
| drop(unsendable); | ||
| }); | ||
| }) | ||
| .join() | ||
| let unsendable = Py::new( | ||
| py, | ||
| Unsendable { | ||
| dropped: dropped.clone(), | ||
| }, | ||
| ) | ||
| .unwrap(); | ||
| }); | ||
|
|
||
| assert!(!dropped.load(Ordering::SeqCst)); | ||
| py.detach(|| { | ||
| spawn(move || { | ||
| Python::attach(move |py| { | ||
| drop(unsendable); | ||
| // On the free-threaded build, dropping an object on its non-origin thread | ||
| // will not immediately drop it because the refcounts need to be merged. | ||
| // | ||
| // Force GC to ensure the drop happens now on the wrong thread. | ||
| py.run(c_str!("import gc; gc.collect()"), None, None) | ||
| .unwrap(); | ||
|
Comment on lines
+657
to
+662
Member
Author
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. In hindsight this should have been obvious why this test was failing; for a while I was worried it was a refcounting bug on PyO3's end. |
||
| }); | ||
| }) | ||
| .join() | ||
| .unwrap(); | ||
| }); | ||
|
|
||
| assert!(!dropped.load(Ordering::SeqCst)); | ||
|
|
||
| capture.take_capture().unwrap() | ||
| }); | ||
|
|
||
| let (err, object) = capture.borrow_mut(py).capture.take().unwrap(); | ||
| assert_eq!(err.to_string(), "RuntimeError: test_class_basics::drop_unsendable_elsewhere::Unsendable is unsendable, but is being dropped on another thread"); | ||
| assert!(object.is_none(py)); | ||
|
|
||
| capture.borrow_mut(py).uninstall(py); | ||
| assert!(object.is_none()); | ||
| }); | ||
| } | ||
|
|
||
|
|
||
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.
It's not obvious to me why this got deleted, any particular reason? There's a similar pattern in
test_exceptions.rsbelow.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.
Hmm it was just awkward to assert that the unraisable hook hadn't been called yet.
I've found a better pattern where the closure gets access to the hook so it can retrieve the value, will push that and restore this.