Skip to content
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

Remove timeout_ms #83

Merged
merged 2 commits into from Dec 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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");
});
}