Fixed race condition on multithreaded test runs

.gitignore

@@ -15,4 +15,4 @@
 flamegraph.svg
 
 # Development data
-/data
+/dev

src/game/test.rs

@@ -12,9 +12,9 @@ use anyhow::Result;
 use petgraph::dot::{Config, Dot};
 
 use std::fmt::Display;
-use std::fs::create_dir;
 use std::fs::File;
 use std::io::Write;
+use std::path::PathBuf;
 use std::process::{Command, Stdio};
 
 use crate::game::mock;
@@ -32,14 +32,9 @@ impl mock::Session<'_> {
             TestSetting::Development => (),
         }
 
+        let subdir = PathBuf::from(module);
+        let mut dir = get_directory(DevelopmentData::Visuals, subdir)?;
         let name = format!("{}.svg", self.name()).replace(" ", "-");
-        let mut dir = get_directory(TestData::Visuals)?;
-
-        dir.push(module);
-        if !dir.exists() {
-            create_dir(&dir)
-                .context("Failed to create module subdirectory.")?;
-        }
 
         dir.push(name);
         let file = File::create(dir)?;

src/game/zero_by/mod.rs

@@ -10,7 +10,6 @@
 //! emulate.
 //!
 //! #### Authorship
-//!
 //! - Max Fierro, 4/6/2023 (maxfierro@berkeley.edu)
 
 use anyhow::{Context, Result};

src/game/zero_by/states.rs

@@ -5,7 +5,6 @@
 //! partially ensure compatibility with a game variant.
 //!
 //! #### Authorship
-//!
 //! - Max Fierro, 11/2/2023 (maxfierro@berkeley.edu)
 
 use regex::Regex;

src/game/zero_by/variants.rs

@@ -4,7 +4,6 @@
 //! into parameters that can help build a game session.
 //!
 //! #### Authorship
-//!
 //! - Max Fierro, 11/2/2023 (maxfierro@berkeley.edu)
 
 use regex::Regex;

src/test.rs

@@ -10,19 +10,31 @@
 use anyhow::{anyhow, Context, Result};
 use strum_macros::Display;
 
-use std::{env, fs, path};
+use std::{
+    env, fs,
+    path::{self, PathBuf},
+    sync::RwLock,
+};
 
 /* CONSTANTS */
 
-const DATA_DIRECTORY: &str = "data";
+/// Global lock for creating development data directories. Since `cargo test`
+/// executes tests in parallel, this helps prevent test flakiness by avoiding
+/// race conditions when creating directories (specific files are still
+/// susceptible, but they have no specific structure).
+pub static DIRECTORY_LOCK: RwLock<()> = RwLock::new(());
+
+/// The name of the global directory at the project root used for generated
+/// development data. This directory is not shipped with release builds.
+const DEV_DIRECTORY: &str = "dev";
 
 /* DEFINITIONS */
 
 /// Specifies directories for different kinds of data generated for development
 /// purposes, which should not be distributed.
 #[derive(Display)]
 #[strum(serialize_all = "kebab-case")]
-pub enum TestData {
+pub enum DevelopmentData {
     Visuals,
 }
 
@@ -55,33 +67,47 @@ pub fn test_setting() -> Result<TestSetting> {
 }
 
 /// Returns a PathBuf corresponding to the correct subdirectory for storing
-/// development `data`, creating it in the process if it does not exist.
-pub fn get_directory(data: TestData) -> Result<path::PathBuf> {
+/// development `data` at a `module`-specific subdirectory, creating it in the
+/// process if it does not exist.
+pub fn get_directory(
+    data: DevelopmentData,
+    module: PathBuf,
+) -> Result<path::PathBuf> {
     let root = find_cargo_lock_directory()
         .context("Failed to find project root directory.")?;
 
-    let data_dir = root.join(DATA_DIRECTORY);
-    if !data_dir.exists() {
-        fs::create_dir(&data_dir)
-            .context("Failed to create data directory at project root.")?;
-    }
-    let name = format!("{}", data);
-    let dir = data_dir.join(name);
-    if !dir.exists() {
-        fs::create_dir(&dir)
-            .context("Failed to create subdirectory inside data directory.")?;
+    let directory = root
+        .join(DEV_DIRECTORY)
+        .join(format!("{}", data))
+        .join(module);
+
+    let guard = {
+        let _lock = DIRECTORY_LOCK.read().unwrap();
+        directory.try_exists()?
+    };
+
+    if !guard {
+        // Does not completely prevent multiple threads from attempting to
+        // create the same directory path, but `create_dir_all` is resilient
+        // to this regardless. This is only necessary for preventing race
+        // conditions within `find_cargo_lock_directory`.
+        let _lock = DIRECTORY_LOCK.write().unwrap();
+        fs::create_dir_all(&directory)
+            .context("Failed to create module subdirectory.")?;
     }
-    Ok(dir)
+
+    Ok(directory)
 }
 
 /* HELPER FUNCTIONS */
 
 /// Searches for a parent directory containing a `Cargo.lock` file.
 fn find_cargo_lock_directory() -> Result<path::PathBuf> {
+    let _lock = DIRECTORY_LOCK.read().unwrap();
     let mut cwd = env::current_dir()?;
     loop {
-        let lock_file = cwd.join("Cargo.lock");
-        if lock_file.exists() && lock_file.is_file() {
+        let cargo_lock = cwd.join("Cargo.lock");
+        if cargo_lock.try_exists()? && cargo_lock.is_file() {
             return Ok(cwd);
         }
         if let Some(parent_dir) = cwd.parent() {