Skip to content

Commit

Permalink
Merge pull request #83 from palfrey/remove-timeout
Browse files Browse the repository at this point in the history
Remove timeout_ms
  • Loading branch information
palfrey committed Dec 12, 2022
2 parents 561a97c + 34529be commit cb516b9
Show file tree
Hide file tree
Showing 10 changed files with 38 additions and 207 deletions.
11 changes: 0 additions & 11 deletions README.md
Expand Up @@ -53,14 +53,3 @@ 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](https://github.com/palfrey/serial_test/issues/76) based on the number of parallel tests run on the system.
```rust
#[test]
#[serial(timeout_ms = 1000)]
fn test_serial_one() {
// Do things
}
```
21 changes: 7 additions & 14 deletions serial_test/src/code_lock.rs
@@ -1,12 +1,11 @@
use crate::rwlock::{Locks, MutexGuardWrapper};
use dashmap::{try_result::TryResult, DashMap};
use lazy_static::lazy_static;
#[cfg(all(feature = "logging"))]
#[cfg(feature = "logging")]
use log::debug;
use std::{
sync::{atomic::AtomicU32, Arc},
time::{Duration, Instant},
};
use std::sync::{atomic::AtomicU32, Arc};
#[cfg(feature = "logging")]
use std::time::Instant;

pub(crate) struct UniqueReentrantMutex {
locks: Locks,
Expand Down Expand Up @@ -55,10 +54,11 @@ impl Default for UniqueReentrantMutex {
}
}

pub(crate) fn check_new_key(name: &str, max_wait: Option<Duration>) {
pub(crate) fn check_new_key(name: &str) {
#[cfg(feature = "logging")]
let start = Instant::now();
loop {
#[cfg(all(feature = "logging"))]
#[cfg(feature = "logging")]
{
let duration = start.elapsed();
debug!("Waiting for '{}' {:?}", name, duration);
Expand All @@ -84,12 +84,5 @@ pub(crate) fn check_new_key(name: &str, max_wait: Option<Duration>) {

// 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

if let Some(max_wait) = max_wait {
let duration = start.elapsed();
if duration > max_wait {
panic!("Timeout waiting for '{}' {:?}", name, duration);
}
}
}
}
12 changes: 0 additions & 12 deletions serial_test/src/lib.rs
Expand Up @@ -46,18 +46,6 @@
//! }
//! ````
//!
//!
//! 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](https://github.com/palfrey/serial_test/issues/76) based on the number of parallel tests run on the system.
//! ```rust
//! #[test]
//! #[serial(timeout_ms = 1000)]
//! fn test_serial_one() {
//! // Do things
//! }
//! ```
//!
//! ## Feature flags
#![cfg_attr(
feature = "docsrs",
Expand Down
22 changes: 8 additions & 14 deletions serial_test/src/parallel_code_lock.rs
Expand Up @@ -3,15 +3,14 @@
use crate::code_lock::{check_new_key, LOCKS};
#[cfg(feature = "async")]
use futures::FutureExt;
use std::{panic, time::Duration};
use std::panic;

#[doc(hidden)]
pub fn local_parallel_core_with_return<E>(
name: &str,
max_wait: Option<Duration>,
function: fn() -> Result<(), E>,
) -> Result<(), E> {
check_new_key(name, max_wait);
check_new_key(name);

let lock = LOCKS.get(name).unwrap();
lock.start_parallel();
Expand All @@ -26,8 +25,8 @@ pub fn local_parallel_core_with_return<E>(
}

#[doc(hidden)]
pub fn local_parallel_core(name: &str, max_wait: Option<Duration>, function: fn()) {
check_new_key(name, max_wait);
pub fn local_parallel_core(name: &str, function: fn()) {
check_new_key(name);

let lock = LOCKS.get(name).unwrap();
lock.start_parallel();
Expand All @@ -44,10 +43,9 @@ pub fn local_parallel_core(name: &str, max_wait: Option<Duration>, function: fn(
#[cfg(feature = "async")]
pub async fn local_async_parallel_core_with_return<E>(
name: &str,
max_wait: Option<Duration>,
fut: impl std::future::Future<Output = Result<(), E>> + panic::UnwindSafe,
) -> Result<(), E> {
check_new_key(name, max_wait);
check_new_key(name);

let lock = LOCKS.get(name).unwrap();
lock.start_parallel();
Expand All @@ -65,10 +63,9 @@ pub async fn local_async_parallel_core_with_return<E>(
#[cfg(feature = "async")]
pub async fn local_async_parallel_core(
name: &str,
max_wait: Option<Duration>,
fut: impl std::future::Future<Output = ()> + panic::UnwindSafe,
) {
check_new_key(name, max_wait);
check_new_key(name);

let lock = LOCKS.get(name).unwrap();
lock.start_parallel();
Expand All @@ -90,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", None, || {
local_parallel_core("unlock_on_assert_sync_without_return", || {
assert!(false);
})
});
Expand All @@ -108,7 +105,6 @@ mod tests {
let _ = panic::catch_unwind(|| {
local_parallel_core_with_return(
"unlock_on_assert_sync_with_return",
None,
|| -> Result<(), Error> {
assert!(false);
Ok(())
Expand All @@ -131,8 +127,7 @@ mod tests {
assert!(false);
}
async fn call_serial_test_fn() {
local_async_parallel_core("unlock_on_assert_async_without_return", None, demo_assert())
.await
local_async_parallel_core("unlock_on_assert_async_without_return", demo_assert()).await
}
// as per https://stackoverflow.com/a/66529014/320546
let _ = panic::catch_unwind(|| {
Expand Down Expand Up @@ -161,7 +156,6 @@ mod tests {
async fn call_serial_test_fn() {
local_async_parallel_core_with_return(
"unlock_on_assert_async_with_return",
None,
demo_assert(),
)
.await;
Expand Down
16 changes: 2 additions & 14 deletions serial_test/src/parallel_file_lock.rs
@@ -1,17 +1,12 @@
use std::{panic, time::Duration};
use std::panic;

#[cfg(feature = "async")]
use futures::FutureExt;

use crate::file_lock::make_lock_for_name_and_path;

#[doc(hidden)]
pub fn fs_parallel_core(
name: &str,
_max_wait: Option<Duration>,
path: Option<&str>,
function: fn(),
) {
pub fn fs_parallel_core(name: &str, path: Option<&str>, function: fn()) {
make_lock_for_name_and_path(name, path).start_parallel();
let res = panic::catch_unwind(|| {
function();
Expand All @@ -25,7 +20,6 @@ pub fn fs_parallel_core(
#[doc(hidden)]
pub fn fs_parallel_core_with_return<E>(
name: &str,
_max_wait: Option<Duration>,
path: Option<&str>,
function: fn() -> Result<(), E>,
) -> Result<(), E> {
Expand All @@ -44,7 +38,6 @@ pub fn fs_parallel_core_with_return<E>(
#[cfg(feature = "async")]
pub async fn fs_async_parallel_core_with_return<E>(
name: &str,
_max_wait: Option<Duration>,
path: Option<&str>,
fut: impl std::future::Future<Output = Result<(), E>> + panic::UnwindSafe,
) -> Result<(), E> {
Expand All @@ -63,7 +56,6 @@ pub async fn fs_async_parallel_core_with_return<E>(
#[cfg(feature = "async")]
pub async fn fs_async_parallel_core(
name: &str,
_max_wait: Option<Duration>,
path: Option<&str>,
fut: impl std::future::Future<Output = ()> + panic::UnwindSafe,
) {
Expand Down Expand Up @@ -97,7 +89,6 @@ mod tests {
let _ = panic::catch_unwind(|| {
fs_parallel_core(
"unlock_on_assert_sync_without_return",
None,
Some(&lock_path),
|| {
assert!(false);
Expand All @@ -113,7 +104,6 @@ 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);
Expand All @@ -134,7 +124,6 @@ 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(),
)
Expand Down Expand Up @@ -164,7 +153,6 @@ 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(),
)
Expand Down
23 changes: 8 additions & 15 deletions serial_test/src/serial_code_lock.rs
@@ -1,15 +1,13 @@
#![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<E>(
name: &str,
max_wait: Option<Duration>,
function: fn() -> Result<(), E>,
) -> Result<(), E> {
check_new_key(name, max_wait);
check_new_key(name);

let unlock = LOCKS.get(name).expect("key to be set");
// _guard needs to be named to avoid being instant dropped
Expand All @@ -18,8 +16,8 @@ pub fn local_serial_core_with_return<E>(
}

#[doc(hidden)]
pub fn local_serial_core(name: &str, max_wait: Option<Duration>, function: fn()) {
check_new_key(name, max_wait);
pub fn local_serial_core(name: &str, function: fn()) {
check_new_key(name);

let unlock = LOCKS.get(name).expect("key to be set");
// _guard needs to be named to avoid being instant dropped
Expand All @@ -31,10 +29,9 @@ pub fn local_serial_core(name: &str, max_wait: Option<Duration>, function: fn())
#[cfg(feature = "async")]
pub async fn local_async_serial_core_with_return<E>(
name: &str,
max_wait: Option<Duration>,
fut: impl std::future::Future<Output = Result<(), E>>,
) -> Result<(), E> {
check_new_key(name, max_wait);
check_new_key(name);

let unlock = LOCKS.get(name).expect("key to be set");
// _guard needs to be named to avoid being instant dropped
Expand All @@ -44,12 +41,8 @@ pub async fn local_async_serial_core_with_return<E>(

#[doc(hidden)]
#[cfg(feature = "async")]
pub async fn local_async_serial_core(
name: &str,
max_wait: Option<Duration>,
fut: impl std::future::Future<Output = ()>,
) {
check_new_key(name, max_wait);
pub async fn local_async_serial_core(name: &str, fut: impl std::future::Future<Output = ()>) {
check_new_key(name);

let unlock = LOCKS.get(name).expect("key to be set");
// _guard needs to be named to avoid being instant dropped
Expand Down Expand Up @@ -85,7 +78,7 @@ mod tests {
let c = barrier.clone();
threads.push(thread::spawn(move || {
c.wait();
check_new_key("foo", None);
check_new_key("foo");
{
let unlock = local_locks.get("foo").expect("read didn't work");
let mutex = unlock.value();
Expand Down Expand Up @@ -113,7 +106,7 @@ mod tests {
#[test]
fn unlock_on_assert() {
let _ = std::panic::catch_unwind(|| {
local_serial_core("assert", None, || {
local_serial_core("assert", || {
assert!(false);
})
});
Expand Down
11 changes: 3 additions & 8 deletions serial_test/src/serial_file_lock.rs
@@ -1,9 +1,7 @@
use std::time::Duration;

use crate::file_lock::make_lock_for_name_and_path;

#[doc(hidden)]
pub fn fs_serial_core(name: &str, _max_wait: Option<Duration>, path: Option<&str>, function: fn()) {
pub fn fs_serial_core(name: &str, path: Option<&str>, function: fn()) {
let mut lock = make_lock_for_name_and_path(name, path);
lock.start_serial();
function();
Expand All @@ -13,7 +11,6 @@ pub fn fs_serial_core(name: &str, _max_wait: Option<Duration>, path: Option<&str
#[doc(hidden)]
pub fn fs_serial_core_with_return<E>(
name: &str,
_max_wait: Option<Duration>,
path: Option<&str>,
function: fn() -> Result<(), E>,
) -> Result<(), E> {
Expand All @@ -28,7 +25,6 @@ pub fn fs_serial_core_with_return<E>(
#[cfg(feature = "async")]
pub async fn fs_async_serial_core_with_return<E>(
name: &str,
_max_wait: Option<Duration>,
path: Option<&str>,
fut: impl std::future::Future<Output = Result<(), E>>,
) -> Result<(), E> {
Expand All @@ -43,7 +39,6 @@ pub async fn fs_async_serial_core_with_return<E>(
#[cfg(feature = "async")]
pub async fn fs_async_serial_core(
name: &str,
_max_wait: Option<Duration>,
path: Option<&str>,
fut: impl std::future::Future<Output = ()>,
) {
Expand All @@ -64,14 +59,14 @@ mod tests {

#[test]
fn test_serial() {
fs_serial_core("test", None, None, || {});
fs_serial_core("test", 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", None, Some(&lock_path), || {
fs_serial_core("foo", Some(&lock_path), || {
assert!(false);
})
});
Expand Down
2 changes: 1 addition & 1 deletion serial_test/tests/tests.rs
Expand Up @@ -2,7 +2,7 @@ use serial_test::local_serial_core;

#[test]
fn test_empty_serial_call() {
local_serial_core("beta", None, || {
local_serial_core("beta", || {
println!("Bar");
});
}

0 comments on commit cb516b9

Please sign in to comment.