From 60f6f61ac99b8f2a6bf5911cc02cb534e71eb019 Mon Sep 17 00:00:00 2001 From: Alexander Batischev Date: Sun, 18 Dec 2016 15:35:30 +0300 Subject: [PATCH 1/2] tests/chmod: don't make assumptions about umask `test_chmod_many_options` relied on user's umask not denying read access for anyone. 022, which is the default umask for many, indeed allows read access for everyone. I'm using 027, which disallows read for everyone but owner and group. This made tests fail. Now tests set and reset umask, ensuring checks are run in a reliable, predictable environment. --- tests/test_chmod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_chmod.rs b/tests/test_chmod.rs index b7f0e8e8c..0695b541e 100644 --- a/tests/test_chmod.rs +++ b/tests/test_chmod.rs @@ -117,10 +117,16 @@ fn test_chmod_ugo_copy() { #[test] fn test_chmod_many_options() { + let original_umask = unsafe { + umask(0) + }; let tests = vec!{ TestCase{args: vec!{"-r,a+w", TEST_FILE}, before: 0o444, after: 0o222}, }; run_tests(tests); + unsafe { + umask(original_umask); + } } #[test] From 655804cff4c5ebc4bf7b62e259259fae4b12ee4b Mon Sep 17 00:00:00 2001 From: Alexander Batischev Date: Mon, 19 Dec 2016 13:02:58 +0300 Subject: [PATCH 2/2] tests/chmod: protect umask with a mutex `test_chmod_ugoa` and `test_chmod_many_options` both change umask, which is global state. Since tests run concurrently, this might lead to a situation where one of the tests changes umask to a value that screws another test's checks. To prevent this, we introduce a mutex that should be held by any test that changes umask. Unfortunately, there's no way to hide umask behind this mutex and enforce its usage: programmers will have to maintain the discipline themselves. --- Cargo.lock | 1 + Cargo.toml | 1 + tests/test_chmod.rs | 8 ++++++++ tests/tests.rs | 3 +++ 4 files changed, 13 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 613f6136e..c0cf8d330 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -35,6 +35,7 @@ dependencies = [ "id 0.0.1", "install 0.0.1", "kill 0.0.1", + "lazy_static 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)", "link 0.0.1", "ln 0.0.1", diff --git a/Cargo.toml b/Cargo.toml index 69741eaa7..5fd1a295b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -233,6 +233,7 @@ regex="*" rand="*" tempdir="*" unindent="*" +lazy_static = "*" [[bin]] name = "uutils" diff --git a/tests/test_chmod.rs b/tests/test_chmod.rs index 0695b541e..9a81da92f 100644 --- a/tests/test_chmod.rs +++ b/tests/test_chmod.rs @@ -1,6 +1,7 @@ use common::util::*; use std::fs::{metadata, OpenOptions, set_permissions}; use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; +use std::sync::Mutex; extern crate libc; use self::libc::umask; @@ -9,6 +10,9 @@ use self::libc::umask; static TEST_FILE: &'static str = "file"; static REFERENCE_FILE: &'static str = "reference"; static REFERENCE_PERMS: u32 = 0o247; +lazy_static! { + static ref UMASK_MUTEX: Mutex<()> = Mutex::new(()); +} struct TestCase { args: Vec<&'static str>, @@ -70,6 +74,8 @@ fn test_chmod_octal() { #[test] fn test_chmod_ugoa() { + let _guard = UMASK_MUTEX.lock(); + let last = unsafe { umask(0) }; @@ -117,6 +123,8 @@ fn test_chmod_ugo_copy() { #[test] fn test_chmod_many_options() { + let _guard = UMASK_MUTEX.lock(); + let original_umask = unsafe { umask(0) }; diff --git a/tests/tests.rs b/tests/tests.rs index 3897935e2..b4b16c13e 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1,6 +1,9 @@ #[macro_use] mod common; +#[macro_use] +extern crate lazy_static; + // For conditional compilation macro_rules! unix_only { ($($fea:expr, $m:ident);+) => {