From c561074425cef41265266fd9056f9ac853f5f20e Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 23 May 2023 15:45:04 -0700 Subject: [PATCH 1/3] mkdir: explicitly set umask when testing mode --- tests/by-util/test_mkdir.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/by-util/test_mkdir.rs b/tests/by-util/test_mkdir.rs index efa7c3248..ba29c31ca 100644 --- a/tests/by-util/test_mkdir.rs +++ b/tests/by-util/test_mkdir.rs @@ -162,9 +162,14 @@ fn test_symbolic_mode() { fn test_symbolic_alteration() { let (at, mut ucmd) = at_and_ucmd!(); + let default_umask = 0o022; + let original_umask = unsafe { umask(default_umask) }; + ucmd.arg("-m").arg("-w").arg(TEST_DIR1).succeeds(); let perms = at.metadata(TEST_DIR1).permissions().mode(); assert_eq!(perms, 0o40577); + + unsafe { umask(original_umask) }; } #[test] From f8a5dbc41c6e117c34ce5ca7132674c7627a8748 Mon Sep 17 00:00:00 2001 From: John Shin Date: Tue, 23 May 2023 15:56:51 -0700 Subject: [PATCH 2/3] mkdir: run tests sequentially by adding a mutex --- tests/by-util/test_mkdir.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/by-util/test_mkdir.rs b/tests/by-util/test_mkdir.rs index ba29c31ca..308df4805 100644 --- a/tests/by-util/test_mkdir.rs +++ b/tests/by-util/test_mkdir.rs @@ -1,8 +1,12 @@ use crate::common::util::TestScenario; #[cfg(not(windows))] use libc::{mode_t, umask}; +use once_cell::sync::Lazy; #[cfg(not(windows))] use std::os::unix::fs::PermissionsExt; +use std::sync::Mutex; + +static TEST_MUTEX: Lazy> = Lazy::new(|| Mutex::new(())); static TEST_DIR1: &str = "mkdir_test1"; static TEST_DIR2: &str = "mkdir_test2"; @@ -20,16 +24,19 @@ static TEST_DIR12: &str = "mkdir_test12"; #[test] fn test_invalid_arg() { + let _guard = TEST_MUTEX.lock(); new_ucmd!().arg("--definitely-invalid").fails().code_is(1); } #[test] fn test_mkdir_mkdir() { + let _guard = TEST_MUTEX.lock(); new_ucmd!().arg(TEST_DIR1).succeeds(); } #[test] fn test_mkdir_verbose() { + let _guard = TEST_MUTEX.lock(); let expected = "mkdir: created directory 'mkdir_test1'\n"; new_ucmd!() .arg(TEST_DIR1) @@ -40,6 +47,7 @@ fn test_mkdir_verbose() { #[test] fn test_mkdir_dup_dir() { + let _guard = TEST_MUTEX.lock(); let scene = TestScenario::new(util_name!()); scene.ucmd().arg(TEST_DIR2).succeeds(); scene.ucmd().arg(TEST_DIR2).fails(); @@ -47,11 +55,13 @@ fn test_mkdir_dup_dir() { #[test] fn test_mkdir_mode() { + let _guard = TEST_MUTEX.lock(); new_ucmd!().arg("-m").arg("755").arg(TEST_DIR3).succeeds(); } #[test] fn test_mkdir_parent() { + let _guard = TEST_MUTEX.lock(); let scene = TestScenario::new(util_name!()); scene.ucmd().arg("-p").arg(TEST_DIR4).succeeds(); scene.ucmd().arg("-p").arg(TEST_DIR4).succeeds(); @@ -61,11 +71,13 @@ fn test_mkdir_parent() { #[test] fn test_mkdir_no_parent() { + let _guard = TEST_MUTEX.lock(); new_ucmd!().arg(TEST_DIR5).fails(); } #[test] fn test_mkdir_dup_dir_parent() { + let _guard = TEST_MUTEX.lock(); let scene = TestScenario::new(util_name!()); scene.ucmd().arg(TEST_DIR6).succeeds(); scene.ucmd().arg("-p").arg(TEST_DIR6).succeeds(); @@ -74,6 +86,7 @@ fn test_mkdir_dup_dir_parent() { #[cfg(not(windows))] #[test] fn test_mkdir_parent_mode() { + let _guard = TEST_MUTEX.lock(); let (at, mut ucmd) = at_and_ucmd!(); let default_umask: mode_t = 0o160; @@ -102,6 +115,7 @@ fn test_mkdir_parent_mode() { #[cfg(not(windows))] #[test] fn test_mkdir_parent_mode_check_existing_parent() { + let _guard = TEST_MUTEX.lock(); let (at, mut ucmd) = at_and_ucmd!(); at.mkdir("a"); @@ -139,6 +153,7 @@ fn test_mkdir_parent_mode_check_existing_parent() { #[test] fn test_mkdir_dup_file() { + let _guard = TEST_MUTEX.lock(); let scene = TestScenario::new(util_name!()); scene.fixtures.touch(TEST_FILE7); scene.ucmd().arg(TEST_FILE7).fails(); @@ -150,6 +165,7 @@ fn test_mkdir_dup_file() { #[test] #[cfg(not(windows))] fn test_symbolic_mode() { + let _guard = TEST_MUTEX.lock(); let (at, mut ucmd) = at_and_ucmd!(); ucmd.arg("-m").arg("a=rwx").arg(TEST_DIR1).succeeds(); @@ -160,6 +176,7 @@ fn test_symbolic_mode() { #[test] #[cfg(not(windows))] fn test_symbolic_alteration() { + let _guard = TEST_MUTEX.lock(); let (at, mut ucmd) = at_and_ucmd!(); let default_umask = 0o022; @@ -175,6 +192,7 @@ fn test_symbolic_alteration() { #[test] #[cfg(not(windows))] fn test_multi_symbolic() { + let _guard = TEST_MUTEX.lock(); let (at, mut ucmd) = at_and_ucmd!(); ucmd.arg("-m") @@ -187,6 +205,7 @@ fn test_multi_symbolic() { #[test] fn test_recursive_reporting() { + let _guard = TEST_MUTEX.lock(); new_ucmd!() .arg("-p") .arg("-v") @@ -208,6 +227,7 @@ fn test_recursive_reporting() { #[test] fn test_mkdir_trailing_dot() { + let _guard = TEST_MUTEX.lock(); let scene2 = TestScenario::new("ls"); new_ucmd!() .arg("-p") @@ -236,6 +256,7 @@ fn test_mkdir_trailing_dot() { #[cfg(not(windows))] fn test_umask_compliance() { fn test_single_case(umask_set: mode_t) { + let _guard = TEST_MUTEX.lock(); let (at, mut ucmd) = at_and_ucmd!(); let original_umask = unsafe { umask(umask_set) }; From 90ed91608eabae6fbab01e42f333a685934d2cc5 Mon Sep 17 00:00:00 2001 From: John Shin Date: Wed, 24 May 2023 16:50:08 -0700 Subject: [PATCH 3/3] mkdir: document why a mutex is necessary --- tests/by-util/test_mkdir.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/by-util/test_mkdir.rs b/tests/by-util/test_mkdir.rs index 308df4805..425be8c35 100644 --- a/tests/by-util/test_mkdir.rs +++ b/tests/by-util/test_mkdir.rs @@ -6,6 +6,10 @@ use once_cell::sync::Lazy; use std::os::unix::fs::PermissionsExt; use std::sync::Mutex; +// tests in `test_mkdir.rs` cannot run in parallel since some tests alter the umask. This may cause +// other tests to run under a wrong set of permissions +// +// when writing a test case, acquire this mutex before proceeding with the main logic of the test static TEST_MUTEX: Lazy> = Lazy::new(|| Mutex::new(())); static TEST_DIR1: &str = "mkdir_test1";