From 8b89a13b90a69132d9e763f30ebc280197b08fcb Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Mon, 10 Apr 2023 20:48:44 +0200 Subject: [PATCH] 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 | 8 ++++--- src/library/beets.rs | 41 ++++++++++++++++++++++++++--------- src/main.rs | 2 +- tests/database/json.rs | 13 +++++------ tests/library/beets.rs | 49 ++++++++++++++++++++++++++---------------- 5 files changed, 74 insertions(+), 39 deletions(-) diff --git a/src/database/json.rs b/src/database/json.rs index a940d9f..d6ad01b 100644 --- a/src/database/json.rs +++ b/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, + backend: Box, } impl DatabaseJson { - pub fn new(backend: Box) -> Self { + /// Create a new JSON database with the provided executor, e.g. [DatabaseJsonFile]. + pub fn new(backend: Box) -> 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, } diff --git a/src/library/beets.rs b/src/library/beets.rs index 98d9a5d..065147a 100644 --- a/src/library/beets.rs +++ b/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, + executor: Box, } trait LibraryPrivate { @@ -106,7 +106,8 @@ trait LibraryPrivate { } impl Beets { - pub fn new(executor: Box) -> Beets { + /// Create a new beets library instance with the provided executor, e.g. [SystemExecutor]. + pub fn new(executor: Box) -> 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, } 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, Error> { let mut cmd = Command::new(&self.bin); diff --git a/src/main.rs b/src/main.rs index 0cfcfc8..ee8abe5 100644 --- a/src/main.rs +++ b/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 diff --git a/tests/database/json.rs b/tests/database/json.rs index 8b22aef..d747841 100644 --- a/tests/database/json.rs +++ b/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 = + 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 = vec![]; diff --git a/tests/library/beets.rs b/tests/library/beets.rs index f176fe7..a290500 100644 --- a/tests/library/beets.rs +++ b/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>> = Lazy::new(|| { + Arc::new(Mutex::new(Beets::new(Box::new(unsafe { + SystemExecutor::default() + })))) +}); + +static BEETS_TEST_CONFIG: Lazy>> = 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 = 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 = 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();