From fdadd414509837d7503ad4c3dda2e3f98502f878 Mon Sep 17 00:00:00 2001 From: "Heinz N. Gies" Date: Wed, 29 Jun 2022 14:13:14 +0200 Subject: [PATCH 1/9] Add parameter for timeout on a per test level Signed-off-by: Heinz N. Gies --- serial_test/src/code_lock.rs | 32 +------ serial_test/src/lib.rs | 2 - serial_test/src/parallel_code_lock.rs | 22 +++-- serial_test/src/serial_code_lock.rs | 23 +++-- serial_test/tests/tests.rs | 2 +- serial_test_derive/src/lib.rs | 125 ++++++++++++++++---------- 6 files changed, 111 insertions(+), 95 deletions(-) diff --git a/serial_test/src/code_lock.rs b/serial_test/src/code_lock.rs index 1fea4da..7f88ad3 100644 --- a/serial_test/src/code_lock.rs +++ b/serial_test/src/code_lock.rs @@ -4,7 +4,6 @@ use lazy_static::lazy_static; #[cfg(all(feature = "logging", feature = "timeout"))] use log::debug; #[cfg(feature = "timeout")] -use parking_lot::RwLock; use std::sync::{atomic::AtomicU32, Arc}; #[cfg(feature = "timeout")] use std::time::Duration; @@ -49,11 +48,6 @@ lazy_static! { static ref MUTEX_ID: Arc = Arc::new(AtomicU32::new(1)); } -#[cfg(feature = "timeout")] -lazy_static! { - static ref MAX_WAIT: Arc> = Arc::new(RwLock::new(Duration::from_secs(60))); -} - impl Default for UniqueReentrantMutex { fn default() -> Self { Self { @@ -63,27 +57,7 @@ impl Default for UniqueReentrantMutex { } } -/// Sets the maximum amount of time the serial locks will wait to unlock. -/// By default, this is set to 60 seconds, which is almost always much longer than is needed. -/// This is deliberately set high to try and avoid situations where we accidentally hit the limits -/// but is set at all so we can timeout rather than hanging forever. -/// -/// However, sometimes if you've got a *lot* of serial tests it might theoretically not be enough, -/// hence this method. -/// -/// This function is only available when the `timeout` feature is enabled. -#[cfg(feature = "timeout")] -pub fn set_max_wait(max_wait: Duration) { - *MAX_WAIT.write() = max_wait; -} - -#[cfg(feature = "timeout")] -pub(crate) fn wait_duration() -> Duration { - *MAX_WAIT.read() -} - -pub(crate) fn check_new_key(name: &str) { - #[cfg(feature = "timeout")] +pub(crate) fn check_new_key(name: &str, max_wait: Option) { let start = Instant::now(); loop { #[cfg(all(feature = "logging", feature = "timeout"))] @@ -114,9 +88,9 @@ pub(crate) fn check_new_key(name: &str) { // Odds are another test was also locking on the write and has now written the key #[cfg(feature = "timeout")] - { + if let Some(max_wait) = max_wait { let duration = start.elapsed(); - if duration > wait_duration() { + if duration > max_wait { panic!("Timeout waiting for '{}' {:?}", name, duration); } } diff --git a/serial_test/src/lib.rs b/serial_test/src/lib.rs index 838ca9b..9c8c9d7 100644 --- a/serial_test/src/lib.rs +++ b/serial_test/src/lib.rs @@ -64,8 +64,6 @@ mod parallel_file_lock; #[cfg(feature = "file_locks")] mod serial_file_lock; -#[cfg(feature = "timeout")] -pub use code_lock::set_max_wait; pub use parallel_code_lock::{ local_async_parallel_core, local_async_parallel_core_with_return, local_parallel_core, local_parallel_core_with_return, diff --git a/serial_test/src/parallel_code_lock.rs b/serial_test/src/parallel_code_lock.rs index 202cc83..c4cbb73 100644 --- a/serial_test/src/parallel_code_lock.rs +++ b/serial_test/src/parallel_code_lock.rs @@ -2,14 +2,15 @@ use crate::code_lock::{check_new_key, LOCKS}; use futures::FutureExt; -use std::panic; +use std::{panic, time::Duration}; #[doc(hidden)] pub fn local_parallel_core_with_return( name: &str, + max_wait: Option, function: fn() -> Result<(), E>, ) -> Result<(), E> { - check_new_key(name); + check_new_key(name, max_wait); let lock = LOCKS.get(name).unwrap(); lock.start_parallel(); @@ -24,8 +25,8 @@ pub fn local_parallel_core_with_return( } #[doc(hidden)] -pub fn local_parallel_core(name: &str, function: fn()) { - check_new_key(name); +pub fn local_parallel_core(name: &str, max_wait: Option, function: fn()) { + check_new_key(name, max_wait); let lock = LOCKS.get(name).unwrap(); lock.start_parallel(); @@ -41,9 +42,10 @@ pub fn local_parallel_core(name: &str, function: fn()) { #[doc(hidden)] pub async fn local_async_parallel_core_with_return( name: &str, + max_wait: Option, fut: impl std::future::Future> + panic::UnwindSafe, ) -> Result<(), E> { - check_new_key(name); + check_new_key(name, max_wait); let lock = LOCKS.get(name).unwrap(); lock.start_parallel(); @@ -60,9 +62,10 @@ pub async fn local_async_parallel_core_with_return( #[doc(hidden)] pub async fn local_async_parallel_core( name: &str, + max_wait: Option, fut: impl std::future::Future + panic::UnwindSafe, ) { - check_new_key(name); + check_new_key(name, max_wait); let lock = LOCKS.get(name).unwrap(); lock.start_parallel(); @@ -84,7 +87,7 @@ mod tests { #[test] fn unlock_on_assert_sync_without_return() { let _ = panic::catch_unwind(|| { - local_parallel_core("unlock_on_assert_sync_without_return", || { + local_parallel_core("unlock_on_assert_sync_without_return", None, || { assert!(false); }) }); @@ -102,6 +105,7 @@ mod tests { let _ = panic::catch_unwind(|| { local_parallel_core_with_return( "unlock_on_assert_sync_with_return", + None, || -> Result<(), Error> { assert!(false); Ok(()) @@ -123,7 +127,8 @@ mod tests { assert!(false); } async fn call_serial_test_fn() { - local_async_parallel_core("unlock_on_assert_async_without_return", demo_assert()).await + local_async_parallel_core("unlock_on_assert_async_without_return", None, demo_assert()) + .await } // as per https://stackoverflow.com/a/66529014/320546 let _ = panic::catch_unwind(|| { @@ -151,6 +156,7 @@ mod tests { async fn call_serial_test_fn() { local_async_parallel_core_with_return( "unlock_on_assert_async_with_return", + None, demo_assert(), ) .await; diff --git a/serial_test/src/serial_code_lock.rs b/serial_test/src/serial_code_lock.rs index 4a328ca..8071198 100644 --- a/serial_test/src/serial_code_lock.rs +++ b/serial_test/src/serial_code_lock.rs @@ -1,13 +1,15 @@ #![allow(clippy::await_holding_lock)] use crate::code_lock::{check_new_key, LOCKS}; +use std::time::Duration; #[doc(hidden)] pub fn local_serial_core_with_return( name: &str, + max_wait: Option, function: fn() -> Result<(), E>, ) -> Result<(), E> { - check_new_key(name); + check_new_key(name, max_wait); let unlock = LOCKS.get(name).expect("key to be set"); // _guard needs to be named to avoid being instant dropped @@ -16,8 +18,8 @@ pub fn local_serial_core_with_return( } #[doc(hidden)] -pub fn local_serial_core(name: &str, function: fn()) { - check_new_key(name); +pub fn local_serial_core(name: &str, max_wait: Option, function: fn()) { + check_new_key(name, max_wait); let unlock = LOCKS.get(name).expect("key to be set"); // _guard needs to be named to avoid being instant dropped @@ -28,9 +30,10 @@ pub fn local_serial_core(name: &str, function: fn()) { #[doc(hidden)] pub async fn local_async_serial_core_with_return( name: &str, + max_wait: Option, fut: impl std::future::Future>, ) -> Result<(), E> { - check_new_key(name); + check_new_key(name, max_wait); let unlock = LOCKS.get(name).expect("key to be set"); // _guard needs to be named to avoid being instant dropped @@ -39,8 +42,12 @@ pub async fn local_async_serial_core_with_return( } #[doc(hidden)] -pub async fn local_async_serial_core(name: &str, fut: impl std::future::Future) { - check_new_key(name); +pub async fn local_async_serial_core( + name: &str, + max_wait: Option, + fut: impl std::future::Future, +) { + check_new_key(name, max_wait); let unlock = LOCKS.get(name).expect("key to be set"); // _guard needs to be named to avoid being instant dropped @@ -76,7 +83,7 @@ mod tests { let c = barrier.clone(); threads.push(thread::spawn(move || { c.wait(); - check_new_key("foo"); + check_new_key("foo", None); { let unlock = local_locks.get("foo").expect("read didn't work"); let mutex = unlock.value(); @@ -104,7 +111,7 @@ mod tests { #[test] fn unlock_on_assert() { let _ = std::panic::catch_unwind(|| { - local_serial_core("assert", || { + local_serial_core("assert", None, || { assert!(false); }) }); diff --git a/serial_test/tests/tests.rs b/serial_test/tests/tests.rs index add8b10..63213f8 100644 --- a/serial_test/tests/tests.rs +++ b/serial_test/tests/tests.rs @@ -2,7 +2,7 @@ use serial_test::local_serial_core; #[test] fn test_empty_serial_call() { - local_serial_core("beta", || { + local_serial_core("beta", None, || { println!("Bar"); }); } diff --git a/serial_test_derive/src/lib.rs b/serial_test_derive/src/lib.rs index 56bb310..57ab08a 100644 --- a/serial_test_derive/src/lib.rs +++ b/serial_test_derive/src/lib.rs @@ -201,13 +201,37 @@ impl ToTokens for QuoteOption { } } -fn get_raw_args(attr: proc_macro2::TokenStream) -> Vec { +enum Arg { + Name(String), + Timeout(u64), +} + +fn get_raw_args(attr: proc_macro2::TokenStream) -> Vec { let mut attrs = attr.into_iter().collect::>(); - let mut raw_args: Vec = Vec::new(); + let mut raw_args: Vec = Vec::new(); while !attrs.is_empty() { match attrs.remove(0) { TokenTree::Ident(id) => { - raw_args.push(id.to_string()); + let name = id.to_string(); + if name == "timeout" { + match attrs.first() { + Some(TokenTree::Punct(p)) if p.as_char() == '=' && !attrs.is_empty() => { + attrs.remove(0); + if let TokenTree::Literal(lit) = attrs.remove(0) { + let millis = lit + .to_string() + .parse::() + .expect("Not a valid duration for Timeout"); + raw_args.push(Arg::Timeout(millis)); + } else { + panic!("Timeout argument must be a literal duration"); + } + } + _ => raw_args.push(Arg::Name(name)), + } + } else { + raw_args.push(Arg::Name(name)); + } } TokenTree::Literal(literal) => { let string_literal = literal.to_string(); @@ -215,7 +239,9 @@ fn get_raw_args(attr: proc_macro2::TokenStream) -> Vec { panic!("Expected a string literal, got '{}'", string_literal); } // Hacky way of getting a string without the enclosing quotes - raw_args.push(string_literal[1..string_literal.len() - 1].to_string()); + raw_args.push(Arg::Name( + string_literal[1..string_literal.len() - 1].to_string(), + )); } x => { panic!("Expected either strings or literals as args, not {}", x); @@ -233,60 +259,65 @@ fn get_raw_args(attr: proc_macro2::TokenStream) -> Vec { raw_args } -fn get_core_key(attr: proc_macro2::TokenStream) -> String { - let mut raw_args = get_raw_args(attr); - match raw_args.len() { - 0 => "".to_string(), - 1 => raw_args.pop().unwrap(), - n => { - panic!( - "Expected either 0 or 1 arguments, got {}: {:?}", - n, raw_args - ); +#[derive(Default, Debug)] +struct Config { + name: String, + timeout: Option, + path: Option, +} + +fn get_core_key(attr: proc_macro2::TokenStream) -> Config { + let raw_args = get_raw_args(attr); + let mut c = Config::default(); + let mut name_found = false; + for a in raw_args { + match a { + Arg::Name(name) if !name_found => { + c.name = name; + name_found = true; + } + Arg::Name(name) => { + c.path = Some(name); + } + Arg::Timeout(timeout) => { + c.timeout = Some(timeout); + } } } + c } fn local_serial_core( attr: proc_macro2::TokenStream, input: proc_macro2::TokenStream, ) -> proc_macro2::TokenStream { - let key = get_core_key(attr); - serial_setup(input, vec![Box::new(key)], "local") + let config = get_core_key(attr); + let args: Vec> = + vec![Box::new(config.name), Box::new(QuoteOption(config.timeout))]; + serial_setup(input, args, "local") } fn local_parallel_core( attr: proc_macro2::TokenStream, input: proc_macro2::TokenStream, ) -> proc_macro2::TokenStream { - let key = get_core_key(attr); - parallel_setup(input, vec![Box::new(key)], "local") + let config = get_core_key(attr); + let args: Vec> = + vec![Box::new(config.name), Box::new(QuoteOption(config.timeout))]; + parallel_setup(input, args, "local") } fn fs_args(attr: proc_macro2::TokenStream) -> Vec> { - let none_ident = Box::new(format_ident!("None")); - let mut args: Vec> = Vec::new(); - let mut raw_args = get_raw_args(attr); - match raw_args.len() { - 0 => { - args.push(Box::new("".to_string())); - args.push(none_ident); - } - 1 => { - args.push(Box::new(raw_args.pop().unwrap())); - args.push(none_ident); - } - 2 => { - let key = raw_args.remove(0); - let path = raw_args.remove(0); - args.push(Box::new(key)); - args.push(Box::new(QuoteOption(Some(path)))); - } - n => { - panic!("Expected 0-2 arguments, got {}: {:?}", n, raw_args); - } - } - args + let config = get_core_key(attr); + vec![ + Box::new(config.name), + Box::new(QuoteOption(config.timeout)), + if let Some(path) = config.path { + Box::new(QuoteOption(Some(path))) + } else { + Box::new(format_ident!("None")) + }, + ] } fn fs_serial_core( @@ -435,7 +466,7 @@ mod tests { let compare = quote! { #[test] fn foo () { - serial_test::local_serial_core("", || {} ); + serial_test::local_serial_core("", :: std :: option :: Option :: None, || {} ); } }; assert_eq!(format!("{}", compare), format!("{}", stream)); @@ -457,7 +488,7 @@ mod tests { #[test] #[something_else] fn foo () { - serial_test::local_serial_core("", || {} ); + serial_test::local_serial_core("", :: std :: option :: Option :: None, || {} ); } }; assert_eq!(format!("{}", compare), format!("{}", stream)); @@ -472,7 +503,7 @@ mod tests { let stream = local_serial_core(attrs.into(), input); let compare = quote! { async fn foo () { - serial_test::local_async_serial_core("", || async {} ).await; + serial_test::local_async_serial_core("", :: std :: option :: Option :: None, || async {} ).await; } }; assert_eq!(format!("{}", compare), format!("{}", stream)); @@ -487,7 +518,7 @@ mod tests { let stream = local_serial_core(attrs.into(), input); let compare = quote! { async fn foo () -> Result<(), ()> { - serial_test::local_async_serial_core_with_return("", || async { Ok(()) } ).await; + serial_test::local_async_serial_core_with_return("", :: std :: option :: Option :: None, || async { Ok(()) } ).await; } }; assert_eq!(format!("{}", compare), format!("{}", stream)); @@ -515,7 +546,7 @@ mod tests { let compare = quote! { #[test] fn foo () { - serial_test::fs_serial_core("foo", None, || {} ); + serial_test::fs_serial_core("foo", :: std :: option :: Option :: None, None, || {} ); } }; assert_eq!(format!("{}", compare), format!("{}", stream)); @@ -539,7 +570,7 @@ mod tests { let compare = quote! { #[test] fn foo () { - serial_test::fs_serial_core("foo", ::std::option::Option::Some("bar_path"), || {} ); + serial_test::fs_serial_core("foo", :: std :: option :: Option :: None, ::std::option::Option::Some("bar_path"), || {} ); } }; assert_eq!(format!("{}", compare), format!("{}", stream)); From ca40c6cc0e5442c25b7faccbc5af316e8aa549d8 Mon Sep 17 00:00:00 2001 From: "Heinz N. Gies" Date: Fri, 1 Jul 2022 14:46:06 +0200 Subject: [PATCH 2/9] rename config option Signed-off-by: Heinz N. Gies --- serial_test_derive/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serial_test_derive/src/lib.rs b/serial_test_derive/src/lib.rs index 57ab08a..63d4814 100644 --- a/serial_test_derive/src/lib.rs +++ b/serial_test_derive/src/lib.rs @@ -213,7 +213,7 @@ fn get_raw_args(attr: proc_macro2::TokenStream) -> Vec { match attrs.remove(0) { TokenTree::Ident(id) => { let name = id.to_string(); - if name == "timeout" { + if name == "timeout_ms" { match attrs.first() { Some(TokenTree::Punct(p)) if p.as_char() == '=' && !attrs.is_empty() => { attrs.remove(0); From 9f5430259c1a220afc6a2667d537ee5590564025 Mon Sep 17 00:00:00 2001 From: "Heinz N. Gies" Date: Fri, 1 Jul 2022 14:49:53 +0200 Subject: [PATCH 3/9] Remove timeout feature Signed-off-by: Heinz N. Gies --- serial_test/Cargo.toml | 6 +----- serial_test/src/code_lock.rs | 8 ++------ 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/serial_test/Cargo.toml b/serial_test/Cargo.toml index 528fca2..10104c6 100644 --- a/serial_test/Cargo.toml +++ b/serial_test/Cargo.toml @@ -27,7 +27,7 @@ itertools = "0.10" tokio = { version = "^1.17", features = ["macros", "rt"] } [features] -default = ["logging", "timeout"] +default = ["logging"] ## Switches on debug logging (and requires the `log` package) logging = ["log"] @@ -35,10 +35,6 @@ logging = ["log"] ## The file_locks feature unlocks the `file_serial`/`file_parallel` macros file_locks = ["fslock"] -## The `timeout` feature lets tests time out after a certain amount of time -## if not enabled tests will wait indefinitely to be started -timeout = [] - docsrs = ["document-features"] # docs.rs-specific configuration diff --git a/serial_test/src/code_lock.rs b/serial_test/src/code_lock.rs index 7f88ad3..52444fb 100644 --- a/serial_test/src/code_lock.rs +++ b/serial_test/src/code_lock.rs @@ -1,13 +1,10 @@ use crate::rwlock::{Locks, MutexGuardWrapper}; use dashmap::{try_result::TryResult, DashMap}; use lazy_static::lazy_static; -#[cfg(all(feature = "logging", feature = "timeout"))] +#[cfg(all(feature = "logging"))] use log::debug; -#[cfg(feature = "timeout")] use std::sync::{atomic::AtomicU32, Arc}; -#[cfg(feature = "timeout")] use std::time::Duration; -#[cfg(feature = "timeout")] use std::time::Instant; pub(crate) struct UniqueReentrantMutex { @@ -60,7 +57,7 @@ impl Default for UniqueReentrantMutex { pub(crate) fn check_new_key(name: &str, max_wait: Option) { let start = Instant::now(); loop { - #[cfg(all(feature = "logging", feature = "timeout"))] + #[cfg(all(feature = "logging"))] { let duration = start.elapsed(); debug!("Waiting for '{}' {:?}", name, duration); @@ -87,7 +84,6 @@ pub(crate) fn check_new_key(name: &str, max_wait: Option) { // If the try_entry fails, then go around the loop again // Odds are another test was also locking on the write and has now written the key - #[cfg(feature = "timeout")] if let Some(max_wait) = max_wait { let duration = start.elapsed(); if duration > max_wait { From 8854636f79aade6a64d69c2441eafac115027b9d Mon Sep 17 00:00:00 2001 From: "Heinz N. Gies" Date: Wed, 13 Jul 2022 11:34:25 +0200 Subject: [PATCH 4/9] Handle file_lock Signed-off-by: Heinz N. Gies --- serial_test/src/parallel_file_lock.rs | 16 ++++++++++++++-- serial_test/src/serial_file_lock.rs | 11 ++++++++--- serial_test_derive/src/lib.rs | 3 +++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/serial_test/src/parallel_file_lock.rs b/serial_test/src/parallel_file_lock.rs index 1c752bb..dfc3abe 100644 --- a/serial_test/src/parallel_file_lock.rs +++ b/serial_test/src/parallel_file_lock.rs @@ -1,11 +1,16 @@ -use std::panic; +use std::{panic, time::Duration}; use futures::FutureExt; use crate::file_lock::make_lock_for_name_and_path; #[doc(hidden)] -pub fn fs_parallel_core(name: &str, path: Option<&str>, function: fn()) { +pub fn fs_parallel_core( + name: &str, + _max_wait: Option, + path: Option<&str>, + function: fn(), +) { make_lock_for_name_and_path(name, path).start_parallel(); let res = panic::catch_unwind(|| { function(); @@ -19,6 +24,7 @@ pub fn fs_parallel_core(name: &str, path: Option<&str>, function: fn()) { #[doc(hidden)] pub fn fs_parallel_core_with_return( name: &str, + _max_wait: Option, path: Option<&str>, function: fn() -> Result<(), E>, ) -> Result<(), E> { @@ -36,6 +42,7 @@ pub fn fs_parallel_core_with_return( #[doc(hidden)] pub async fn fs_async_parallel_core_with_return( name: &str, + _max_wait: Option, path: Option<&str>, fut: impl std::future::Future> + panic::UnwindSafe, ) -> Result<(), E> { @@ -53,6 +60,7 @@ pub async fn fs_async_parallel_core_with_return( #[doc(hidden)] pub async fn fs_async_parallel_core( name: &str, + _max_wait: Option, path: Option<&str>, fut: impl std::future::Future + panic::UnwindSafe, ) { @@ -84,6 +92,7 @@ mod tests { let _ = panic::catch_unwind(|| { fs_parallel_core( "unlock_on_assert_sync_without_return", + None, Some(&lock_path), || { assert!(false); @@ -99,6 +108,7 @@ mod tests { let _ = panic::catch_unwind(|| { fs_parallel_core_with_return( "unlock_on_assert_sync_with_return", + None, Some(&lock_path), || -> Result<(), Error> { assert!(false); @@ -118,6 +128,7 @@ mod tests { async fn call_serial_test_fn(lock_path: &str) { fs_async_parallel_core( "unlock_on_assert_async_without_return", + None, Some(&lock_path), demo_assert(), ) @@ -146,6 +157,7 @@ mod tests { async fn call_serial_test_fn(lock_path: &str) { fs_async_parallel_core_with_return( "unlock_on_assert_async_with_return", + None, Some(&lock_path), demo_assert(), ) diff --git a/serial_test/src/serial_file_lock.rs b/serial_test/src/serial_file_lock.rs index 23f128c..8c5576e 100644 --- a/serial_test/src/serial_file_lock.rs +++ b/serial_test/src/serial_file_lock.rs @@ -1,7 +1,9 @@ +use std::time::Duration; + use crate::file_lock::make_lock_for_name_and_path; #[doc(hidden)] -pub fn fs_serial_core(name: &str, path: Option<&str>, function: fn()) { +pub fn fs_serial_core(name: &str, _max_wait: Option, path: Option<&str>, function: fn()) { let mut lock = make_lock_for_name_and_path(name, path); lock.start_serial(); function(); @@ -11,6 +13,7 @@ pub fn fs_serial_core(name: &str, path: Option<&str>, function: fn()) { #[doc(hidden)] pub fn fs_serial_core_with_return( name: &str, + _max_wait: Option, path: Option<&str>, function: fn() -> Result<(), E>, ) -> Result<(), E> { @@ -24,6 +27,7 @@ pub fn fs_serial_core_with_return( #[doc(hidden)] pub async fn fs_async_serial_core_with_return( name: &str, + _max_wait: Option, path: Option<&str>, fut: impl std::future::Future>, ) -> Result<(), E> { @@ -37,6 +41,7 @@ pub async fn fs_async_serial_core_with_return( #[doc(hidden)] pub async fn fs_async_serial_core( name: &str, + _max_wait: Option, path: Option<&str>, fut: impl std::future::Future, ) { @@ -57,14 +62,14 @@ mod tests { #[test] fn test_serial() { - fs_serial_core("test", None, || {}); + fs_serial_core("test", None, None, || {}); } #[test] fn unlock_on_assert_sync_without_return() { let lock_path = path_for_name("unlock_on_assert_sync_without_return"); let _ = panic::catch_unwind(|| { - fs_serial_core("foo", Some(&lock_path), || { + fs_serial_core("foo", None, Some(&lock_path), || { assert!(false); }) }); diff --git a/serial_test_derive/src/lib.rs b/serial_test_derive/src/lib.rs index 63d4814..c830b09 100644 --- a/serial_test_derive/src/lib.rs +++ b/serial_test_derive/src/lib.rs @@ -309,6 +309,9 @@ fn local_parallel_core( fn fs_args(attr: proc_macro2::TokenStream) -> Vec> { let config = get_core_key(attr); + if config.timeout.is_some() { + panic!("Timeout is not supported for file_serial"); + } vec![ Box::new(config.name), Box::new(QuoteOption(config.timeout)), From 9f992cbef7790b37a97ce822271085cb5efe8151 Mon Sep 17 00:00:00 2001 From: "Heinz N. Gies" Date: Mon, 18 Jul 2022 07:31:49 +0200 Subject: [PATCH 5/9] Add test Signed-off-by: Heinz N. Gies --- serial_test_derive/src/lib.rs | 49 +++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/serial_test_derive/src/lib.rs b/serial_test_derive/src/lib.rs index c830b09..ad1a212 100644 --- a/serial_test_derive/src/lib.rs +++ b/serial_test_derive/src/lib.rs @@ -475,6 +475,55 @@ mod tests { assert_eq!(format!("{}", compare), format!("{}", stream)); } + #[test] + fn test_serial_with_timeout() { + let attrs = vec![ + TokenTree::Ident(format_ident!("timeout_ms")), + TokenTree::Punct(Punct::new('=', Spacing::Alone)), + TokenTree::Literal(Literal::u8_unsuffixed(42)), + ]; + let input = quote! { + #[test] + fn foo() {} + }; + let stream = local_serial_core( + proc_macro2::TokenStream::from_iter(attrs.into_iter()), + input, + ); + let compare = quote! { + #[test] + fn foo () { + serial_test::local_serial_core("", :: std :: option :: Option :: Some (42u64), || {} ); + } + }; + assert_eq!(format!("{}", compare), format!("{}", stream)); + } + + #[test] + fn test_serial_with_name_and_timeout() { + let attrs = vec![ + TokenTree::Ident(format_ident!("foo")), + TokenTree::Punct(Punct::new(',', Spacing::Alone)), + TokenTree::Ident(format_ident!("timeout_ms")), + TokenTree::Punct(Punct::new('=', Spacing::Alone)), + TokenTree::Literal(Literal::u8_unsuffixed(42)), + ]; + let input = quote! { + #[test] + fn foo() {} + }; + let stream = local_serial_core( + proc_macro2::TokenStream::from_iter(attrs.into_iter()), + input, + ); + let compare = quote! { + #[test] + fn foo () { + serial_test::local_serial_core("foo", :: std :: option :: Option :: Some (42u64), || {} ); + } + }; + assert_eq!(format!("{}", compare), format!("{}", stream)); + } #[test] fn test_stripped_attributes() { let _ = env_logger::builder().is_test(true).try_init(); From 0050cd29be1dba6262b860b966415099c629d95c Mon Sep 17 00:00:00 2001 From: "Heinz N. Gies" Date: Mon, 18 Jul 2022 07:31:56 +0200 Subject: [PATCH 6/9] Update README and docs Signed-off-by: Heinz N. Gies --- README.md | 16 ++++++++++++++++ serial_test_derive/src/lib.rs | 19 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/README.md b/README.md index 49a803e..963d3d5 100644 --- a/README.md +++ b/README.md @@ -53,3 +53,19 @@ extern crate serial_test; for earlier versions. You can then either add `#[serial]` or `#[serial(some_text)]` to tests as required. + +For each test, a timeout can be specified with the `timeout_ms` parameter to the [serial](macro@serial) attribute. Note that +the timeout is counted from the first invocation of the test, not from the time the previous test was completed. This can +lead to some unpredictable behavior based on the number of parallel tests run on the system. +```rust +#[test] +#[serial(timeout_ms = 1000)] +fn test_serial_one() { + // Do things +} +#[test] +#[serial(test_name, timeout_ms = 1000)] +fn test_serial_another() { + // Do things +} +``` \ No newline at end of file diff --git a/serial_test_derive/src/lib.rs b/serial_test_derive/src/lib.rs index ad1a212..23b0446 100644 --- a/serial_test_derive/src/lib.rs +++ b/serial_test_derive/src/lib.rs @@ -33,6 +33,7 @@ use std::ops::Deref; /// If you want different subsets of tests to be serialised with each /// other, but not depend on other subsets, you can add an argument to [serial](macro@serial), and all calls /// with identical arguments will be called in serial. e.g. +/// /// ```` /// #[test] /// #[serial(something)] @@ -61,6 +62,24 @@ use std::ops::Deref; /// `test_serial_one` and `test_serial_another` will be executed in serial, as will `test_serial_third` and `test_serial_fourth` /// but neither sequence will be blocked by the other /// +/// For each test, a timeout can be specified with the `timeout_ms` parameter to the [serial](macro@serial) attribute. Note that +/// the timeout is counted from the first invocation of the test, not from the time the previous test was completed. This can +/// lead to some unpredictable behavior based on the number of parallel tests run on the system. +/// +/// ```` +/// #[test] +/// #[serial(timeout_ms = 1000)] +/// fn test_serial_one() { +/// // Do things +/// } +/// +/// #[test] +/// #[serial(timeout_ms = 1000)] +/// fn test_serial_another() { +/// // Do things +/// } +/// ```` +/// /// Nested serialised tests (i.e. a [serial](macro@serial) tagged test calling another) are supported #[proc_macro_attribute] #[proc_macro_error] From 80bd502ac21ebeff46f0dc5f8d4dbd92d3da64f4 Mon Sep 17 00:00:00 2001 From: "Heinz N. Gies" Date: Fri, 22 Jul 2022 10:25:39 +0200 Subject: [PATCH 7/9] Fix wrong type in escape Signed-off-by: Heinz N. Gies --- serial_test_derive/src/lib.rs | 29 ++++++++++++++++++++--------- serial_test_test/src/lib.rs | 8 ++++++++ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/serial_test_derive/src/lib.rs b/serial_test_derive/src/lib.rs index 23b0446..b1c6d81 100644 --- a/serial_test_derive/src/lib.rs +++ b/serial_test_derive/src/lib.rs @@ -311,8 +311,12 @@ fn local_serial_core( input: proc_macro2::TokenStream, ) -> proc_macro2::TokenStream { let config = get_core_key(attr); - let args: Vec> = - vec![Box::new(config.name), Box::new(QuoteOption(config.timeout))]; + let timeout = if let Some(t) = config.timeout { + quote! { ::std::option::Option::Some(::std::time::Duration::from_millis(#t)) } + } else { + quote! { ::std::option::Option::None } + }; + let args: Vec> = vec![Box::new(config.name), Box::new(timeout)]; serial_setup(input, args, "local") } @@ -321,19 +325,26 @@ fn local_parallel_core( input: proc_macro2::TokenStream, ) -> proc_macro2::TokenStream { let config = get_core_key(attr); - let args: Vec> = - vec![Box::new(config.name), Box::new(QuoteOption(config.timeout))]; + let timeout = if let Some(t) = config.timeout { + quote! { Some(::std::time::Duration::from_millis(#t)) } + } else { + quote! { None } + }; + let args: Vec> = vec![Box::new(config.name), Box::new(timeout)]; parallel_setup(input, args, "local") } fn fs_args(attr: proc_macro2::TokenStream) -> Vec> { let config = get_core_key(attr); - if config.timeout.is_some() { + let timeout = if let Some(_t) = config.timeout { panic!("Timeout is not supported for file_serial"); - } + // quote! { ::std::option::Option::Some(::std::time::Duration::from_millis(#t)) } + } else { + quote! { ::std::option::Option::None } + }; vec![ Box::new(config.name), - Box::new(QuoteOption(config.timeout)), + Box::new(timeout), if let Some(path) = config.path { Box::new(QuoteOption(Some(path))) } else { @@ -512,7 +523,7 @@ mod tests { let compare = quote! { #[test] fn foo () { - serial_test::local_serial_core("", :: std :: option :: Option :: Some (42u64), || {} ); + serial_test::local_serial_core("", :: std :: option :: Option :: Some (::std::time::Duration::from_millis(42u64)), || {} ); } }; assert_eq!(format!("{}", compare), format!("{}", stream)); @@ -538,7 +549,7 @@ mod tests { let compare = quote! { #[test] fn foo () { - serial_test::local_serial_core("foo", :: std :: option :: Option :: Some (42u64), || {} ); + serial_test::local_serial_core("foo", :: std :: option :: Option :: Some (::std::time::Duration::from_millis(42u64)), || {} ); } }; assert_eq!(format!("{}", compare), format!("{}", stream)); diff --git a/serial_test_test/src/lib.rs b/serial_test_test/src/lib.rs index 0150b90..edf84d5 100644 --- a/serial_test_test/src/lib.rs +++ b/serial_test_test/src/lib.rs @@ -106,6 +106,14 @@ mod tests { #[cfg(feature = "file_locks")] use serial_test::{file_parallel, file_serial}; + #[test] + #[serial(timeout_key, timeout_ms = 60000)] + fn demo_timeout_with_key() {} + + #[test] + #[serial(timeout_ms = 60000)] + fn demo_timeout() {} + #[test] #[serial] fn test_serial_no_arg() { From e01c8db7ca48e65d134d73d907c4ee40fd2fad65 Mon Sep 17 00:00:00 2001 From: "Heinz N. Gies" Date: Mon, 25 Jul 2022 09:28:55 +0200 Subject: [PATCH 8/9] Try no timeout Signed-off-by: Heinz N. Gies --- serial_test_test/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/serial_test_test/src/lib.rs b/serial_test_test/src/lib.rs index edf84d5..9b674e6 100644 --- a/serial_test_test/src/lib.rs +++ b/serial_test_test/src/lib.rs @@ -107,11 +107,12 @@ mod tests { use serial_test::{file_parallel, file_serial}; #[test] - #[serial(timeout_key, timeout_ms = 60000)] + #[serial()] fn demo_timeout_with_key() {} #[test] - #[serial(timeout_ms = 60000)] + #[serial()] + // #[serial(timeout_ms = 60000)] fn demo_timeout() {} #[test] From d416977f26982a3486a45a36aa991fa3ec741be1 Mon Sep 17 00:00:00 2001 From: "Heinz N. Gies" Date: Mon, 25 Jul 2022 10:37:13 +0200 Subject: [PATCH 9/9] Re-add timeouts Signed-off-by: Heinz N. Gies --- serial_test/src/code_lock.rs | 7 ++++--- serial_test_test/src/lib.rs | 5 ++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/serial_test/src/code_lock.rs b/serial_test/src/code_lock.rs index 52444fb..ebd728b 100644 --- a/serial_test/src/code_lock.rs +++ b/serial_test/src/code_lock.rs @@ -3,9 +3,10 @@ use dashmap::{try_result::TryResult, DashMap}; use lazy_static::lazy_static; #[cfg(all(feature = "logging"))] use log::debug; -use std::sync::{atomic::AtomicU32, Arc}; -use std::time::Duration; -use std::time::Instant; +use std::{ + sync::{atomic::AtomicU32, Arc}, + time::{Duration, Instant}, +}; pub(crate) struct UniqueReentrantMutex { locks: Locks, diff --git a/serial_test_test/src/lib.rs b/serial_test_test/src/lib.rs index 9b674e6..edf84d5 100644 --- a/serial_test_test/src/lib.rs +++ b/serial_test_test/src/lib.rs @@ -107,12 +107,11 @@ mod tests { use serial_test::{file_parallel, file_serial}; #[test] - #[serial()] + #[serial(timeout_key, timeout_ms = 60000)] fn demo_timeout_with_key() {} #[test] - #[serial()] - // #[serial(timeout_ms = 60000)] + #[serial(timeout_ms = 60000)] fn demo_timeout() {} #[test]