Multiple integration tests can call beets at the same time (#19)

Closes #18

Reviewed-on: https://git.wojciechkozlowski.eu/wojtek/musichoard/pulls/19

src/database/json.rs

@@ -17,13 +17,14 @@ pub trait DatabaseJsonBackend {
     fn write(&mut self, json: &str) -> Result<(), std::io::Error>;
 }
 
-/// A JSON file database.
+/// JSON database.
 pub struct DatabaseJson {
-    backend: Box<dyn DatabaseJsonBackend>,
+    backend: Box<dyn DatabaseJsonBackend + Send>,
 }
 
 impl DatabaseJson {
-    pub fn new(backend: Box<dyn DatabaseJsonBackend>) -> Self {
+    /// Create a new JSON database with the provided executor, e.g. [DatabaseJsonFile].
+    pub fn new(backend: Box<dyn DatabaseJsonBackend + Send>) -> Self {
         DatabaseJson { backend }
     }
 }
@@ -50,6 +51,7 @@ impl DatabaseWrite for DatabaseJson {
     }
 }
 
+/// JSON database that uses a local file for persistent storage.
 pub struct DatabaseJsonFile {
     path: PathBuf,
 }

src/library/beets.rs

@@ -91,7 +91,7 @@ pub trait BeetsExecutor {
 
 /// Struct for interacting with the music library via beets.
 pub struct Beets {
-    executor: Box<dyn BeetsExecutor>,
+    executor: Box<dyn BeetsExecutor + Send>,
 }
 
 trait LibraryPrivate {
@@ -106,7 +106,8 @@ trait LibraryPrivate {
 }
 
 impl Beets {
-    pub fn new(executor: Box<dyn BeetsExecutor>) -> Beets {
+    /// Create a new beets library instance with the provided executor, e.g. [SystemExecutor].
+    pub fn new(executor: Box<dyn BeetsExecutor + Send>) -> Beets {
         Beets { executor }
     }
 }
@@ -236,31 +237,51 @@ impl LibraryPrivate for Beets {
 }
 
 /// Executor for executing beets commands on the local system.
+///
+/// # Safety
+///
+/// The beets executable is not safe to call concurrently for operations on the same
+/// database/library. Therefore, all functions that create a [SystemExecutor] or modify which
+/// library it works with are marked unsafe. It is the caller's responsibility to make sure the
+/// library is not being concurrently accessed from anywhere else.
 pub struct SystemExecutor {
     bin: String,
     config: Option<PathBuf>,
 }
 
 impl SystemExecutor {
-    pub fn new(bin: &str) -> Self {
+    /// Create a new [SystemExecutor] that uses the provided beets executable.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure the library is not being concurrently accessed from anywhere else.
+    pub unsafe fn new(bin: &str) -> Self {
         SystemExecutor {
             bin: bin.to_string(),
             config: None,
         }
     }
 
-    pub fn config(mut self, path: Option<&Path>) -> Self {
+    /// Create a new [SystemExecutor] that uses the system's default beets executable.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure the library is not being concurrently accessed from anywhere else.
+    pub unsafe fn default() -> Self {
+        SystemExecutor::new("beet")
+    }
+
+    /// Update the configuration file for the beets executable.
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure the library is not being concurrently accessed from anywhere else.
+    pub unsafe fn config(mut self, path: Option<&Path>) -> Self {
         self.config = path.map(|p| p.to_path_buf());
         self
     }
 }
 
-impl Default for SystemExecutor {
-    fn default() -> Self {
-        SystemExecutor::new("beet")
-    }
-}
-
 impl BeetsExecutor for SystemExecutor {
     fn exec(&mut self, arguments: &[String]) -> Result<Vec<String>, Error> {
         let mut cmd = Command::new(&self.bin);

src/main.rs

@@ -37,7 +37,7 @@ fn main() {
     let opt = Opt::from_args();
 
     let mut beets = Beets::new(Box::new(
-        SystemExecutor::default().config(opt.beets_config_file_path.as_deref()),
+        unsafe { SystemExecutor::default().config(opt.beets_config_file_path.as_deref()) },
     ));
 
     let collection = beets

tests/database/json.rs

@@ -1,4 +1,4 @@
-use std::fs;
+use std::{fs, path::PathBuf};
 
 use musichoard::{
     database::{
@@ -7,10 +7,14 @@ use musichoard::{
     },
     Artist,
 };
+use once_cell::sync::Lazy;
 use tempfile::NamedTempFile;
 
 use crate::COLLECTION;
 
+static DATABASE_TEST_FILE: Lazy<PathBuf> =
+    Lazy::new(|| fs::canonicalize("./tests/files/database/database.json").unwrap());
+
 #[test]
 fn write() {
     let file = NamedTempFile::new().unwrap();
@@ -21,8 +25,7 @@ fn write() {
     let write_data = COLLECTION.to_owned();
     database.write(&write_data).unwrap();
 
-    let expected_path = fs::canonicalize("./tests/files/database/database.json").unwrap();
-    let expected = fs::read_to_string(expected_path).unwrap();
+    let expected = fs::read_to_string(&*DATABASE_TEST_FILE).unwrap();
     let actual = fs::read_to_string(file.path()).unwrap();
 
     assert_eq!(actual, expected);
@@ -30,9 +33,7 @@ fn write() {
 
 #[test]
 fn read() {
-    let file_path = fs::canonicalize("./tests/files/database/database.json").unwrap();
-
-    let backend = DatabaseJsonFile::new(&file_path);
+    let backend = DatabaseJsonFile::new(&*DATABASE_TEST_FILE);
     let database = DatabaseJson::new(Box::new(backend));
 
     let mut read_data: Vec<Artist> = vec![];

tests/library/beets.rs

@@ -1,4 +1,9 @@
-use std::fs;
+use std::{
+    fs,
+    sync::{Arc, Mutex},
+};
+
+use once_cell::sync::Lazy;
 
 use musichoard::{
     library::{
@@ -10,11 +15,25 @@ use musichoard::{
 
 use crate::COLLECTION;
 
+static BEETS_EMPTY_CONFIG: Lazy<Arc<Mutex<Beets>>> = Lazy::new(|| {
+    Arc::new(Mutex::new(Beets::new(Box::new(unsafe {
+        SystemExecutor::default()
+    }))))
+});
+
+static BEETS_TEST_CONFIG: Lazy<Arc<Mutex<Beets>>> = Lazy::new(|| {
+    Arc::new(Mutex::new(Beets::new(Box::new(unsafe {
+        SystemExecutor::default().config(Some(
+            &fs::canonicalize("./tests/files/library/config.yml").unwrap(),
+        ))
+    }))))
+});
+
 #[test]
 fn test_no_config_list() {
-    let executor = SystemExecutor::default();
+    let beets_arc = BEETS_EMPTY_CONFIG.clone();
+    let beets = &mut beets_arc.lock().unwrap();
 
-    let mut beets = Beets::new(Box::new(executor));
     let output = beets.list(&Query::default()).unwrap();
 
     let expected: Vec<Artist> = vec![];
@@ -23,11 +42,9 @@ fn test_no_config_list() {
 
 #[test]
 fn test_full_list() {
-    let executor = SystemExecutor::default().config(Some(
-        &fs::canonicalize("./tests/files/library/config.yml").unwrap(),
-    ));
+    let beets_arc = BEETS_TEST_CONFIG.clone();
+    let beets = &mut beets_arc.lock().unwrap();
 
-    let mut beets = Beets::new(Box::new(executor));
     let output = beets.list(&Query::default()).unwrap();
 
     let expected: Vec<Artist> = COLLECTION.to_owned();
@@ -36,11 +53,9 @@ fn test_full_list() {
 
 #[test]
 fn test_album_artist_query() {
-    let executor = SystemExecutor::default().config(Some(
-        &fs::canonicalize("./tests/files/library/config.yml").unwrap(),
-    ));
+    let beets_arc = BEETS_TEST_CONFIG.clone();
+    let beets = &mut beets_arc.lock().unwrap();
 
-    let mut beets = Beets::new(Box::new(executor));
     let output = beets
         .list(&Query::default().album_artist(QueryOption::Include(String::from("Аркона"))))
         .unwrap();
@@ -51,11 +66,9 @@ fn test_album_artist_query() {
 
 #[test]
 fn test_album_title_query() {
-    let executor = SystemExecutor::default().config(Some(
-        &fs::canonicalize("./tests/files/library/config.yml").unwrap(),
-    ));
+    let beets_arc = BEETS_TEST_CONFIG.clone();
+    let beets = &mut beets_arc.lock().unwrap();
 
-    let mut beets = Beets::new(Box::new(executor));
     let output = beets
         .list(&Query::default().album_title(QueryOption::Include(String::from("Slovo"))))
         .unwrap();
@@ -66,11 +79,9 @@ fn test_album_title_query() {
 
 #[test]
 fn test_exclude_query() {
-    let executor = SystemExecutor::default().config(Some(
-        &fs::canonicalize("./tests/files/library/config.yml").unwrap(),
-    ));
+    let beets_arc = BEETS_TEST_CONFIG.clone();
+    let beets = &mut beets_arc.lock().unwrap();
 
-    let mut beets = Beets::new(Box::new(executor));
     let output = beets
         .list(&Query::default().album_artist(QueryOption::Exclude(String::from("Аркона"))))
         .unwrap();