Bug 1439582 - Use mutex to ensure tests for set_max_level are not racy. r?jgraham draft
authorAndreas Tolfsen <ato@sny.no>
Wed, 21 Feb 2018 13:41:26 +0000
changeset 757895 a5f560fadd6dca0607447913b28bcd7596c58c66
parent 757856 dd6caba141428343fb26f3ec23a3ee1844a4b241
push id99869
push userbmo:ato@sny.no
push dateWed, 21 Feb 2018 13:45:58 +0000
reviewersjgraham
bugs1439582
milestone60.0a1
Bug 1439582 - Use mutex to ensure tests for set_max_level are not racy. r?jgraham When tests are run in parallel threads, it is possible for tests that use logging::set_max_level to cause race conditions in other tests. This patch ensures that the logging tests that use this function are guarded by a mutex. For example, when set_max_level(Level::Info) is called in test_max_level, test_set_max_level may have also called set_max_level(Level::Fatal). This causes a race condition between the first line and the second line in test_max_level because the atomic test level will have been changed away from Level::Info to Level::Fatal. MozReview-Commit-ID: KQUpkYx6CCo
testing/geckodriver/src/logging.rs
--- a/testing/geckodriver/src/logging.rs
+++ b/testing/geckodriver/src/logging.rs
@@ -183,20 +183,25 @@ pub fn set_max_level(level: Level) {
 
 /// Produces a 13-digit Unix Epoch timestamp similar to Gecko.
 fn format_ts(ts: chrono::DateTime<chrono::Local>) -> String {
     format!("{}{:03}", ts.timestamp(), ts.timestamp_subsec_millis())
 }
 
 #[cfg(test)]
 mod tests {
-    use super::{Level, format_ts, init, init_with_level, max_level, set_max_level};
+    use super::{format_ts, init_with_level, max_level, set_max_level, Level};
     use chrono;
     use log;
     use std::str::FromStr;
+    use std::sync::Mutex;
+
+    lazy_static! {
+        static ref LEVEL_MUTEX: Mutex<()> = Mutex::new(());
+    }
 
     #[test]
     fn test_level_repr() {
         assert_eq!(Level::Fatal as usize, 70);
         assert_eq!(Level::Error as usize, 60);
         assert_eq!(Level::Warn as usize, 50);
         assert_eq!(Level::Info as usize, 40);
         assert_eq!(Level::Config as usize, 30);
@@ -258,30 +263,33 @@ mod tests {
         assert_eq!(Level::Info.to_string(), "INFO");
         assert_eq!(Level::Config.to_string(), "CONFIG");
         assert_eq!(Level::Debug.to_string(), "DEBUG");
         assert_eq!(Level::Trace.to_string(), "TRACE");
     }
 
     #[test]
     fn test_max_level() {
+        let _guard = LEVEL_MUTEX.lock();
         set_max_level(Level::Info);
         assert_eq!(max_level(), Level::Info);
     }
 
     #[test]
     fn test_set_max_level() {
+        let _guard = LEVEL_MUTEX.lock();
         set_max_level(Level::Error);
         assert_eq!(max_level(), Level::Error);
         set_max_level(Level::Fatal);
         assert_eq!(max_level(), Level::Fatal);
     }
 
     #[test]
     fn test_init_with_level() {
+        let _guard = LEVEL_MUTEX.lock();
         init_with_level(Level::Debug).unwrap();
         assert_eq!(max_level(), Level::Debug);
         assert!(init_with_level(Level::Warn).is_err());
     }
 
     #[test]
     fn test_format_ts() {
         let ts = chrono::Local::now();