From 369b4ecf98be0253123ac3d0b728738b17f6568d Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Mon, 23 Sep 2024 22:01:56 +0200 Subject: [PATCH] app complete coverage --- src/tui/app/machine/fetch_state.rs | 111 +++++++++++++- src/tui/app/machine/input.rs | 10 +- src/tui/app/machine/match_state.rs | 136 +++++++++++++++-- src/tui/app/machine/mod.rs | 10 +- .../lib/external/musicbrainz/daemon/mod.rs | 138 ++++++++++++++---- 5 files changed, 353 insertions(+), 52 deletions(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index e2787c5..4073a04 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -19,12 +19,12 @@ use crate::tui::{ }, }; +pub type FetchReceiver = mpsc::Receiver; pub struct FetchState { fetch_rx: FetchReceiver, lookup_rx: Option, } -pub type FetchReceiver = mpsc::Receiver; impl FetchState { pub fn new(fetch_rx: FetchReceiver) -> Self { FetchState { @@ -38,7 +38,7 @@ impl FetchState { let result = lookup_rx.try_recv(); match result { Ok(_) | Err(TryRecvError::Empty) => return result, - _ => { + Err(TryRecvError::Disconnected) => { self.lookup_rx.take(); } } @@ -214,12 +214,47 @@ mod tests { machine::tests::{inner, music_hoard}, Delta, IApp, IAppAccess, IAppInteractBrowse, MatchStateInfo, MissOption, SearchOption, }, - lib::interface::musicbrainz::{self, api::Match, daemon::MockIMbJobSender}, + lib::interface::musicbrainz::{ + self, + api::{Lookup, Match}, + daemon::MockIMbJobSender, + }, testmod::COLLECTION, }; use super::*; + fn mbid() -> Mbid { + "00000000-0000-0000-0000-000000000000".try_into().unwrap() + } + + #[test] + fn try_recv() { + let (fetch_tx, fetch_rx) = mpsc::channel(); + let (lookup_tx, lookup_rx) = mpsc::channel(); + + let mut fetch = FetchState::new(fetch_rx); + fetch.lookup_rx.replace(lookup_rx); + + let artist = COLLECTION[3].meta.clone(); + + let matches: Vec> = vec![]; + let fetch_result = MatchStateInfo::artist_search(artist.clone(), matches); + fetch_tx.send(Ok(fetch_result.clone())).unwrap(); + + assert_eq!(fetch.try_recv(), Err(TryRecvError::Empty)); + + let lookup = Lookup::new(artist.clone()); + let lookup_result = MatchStateInfo::artist_lookup(artist.clone(), lookup); + lookup_tx.send(Ok(lookup_result.clone())).unwrap(); + + assert_eq!(fetch.try_recv(), Ok(Ok(lookup_result))); + + assert_eq!(fetch.try_recv(), Err(TryRecvError::Empty)); + drop(lookup_tx); + assert_eq!(fetch.try_recv(), Ok(Ok(fetch_result))); + } + #[test] fn fetch_no_artist() { let app = AppMachine::app_fetch_new(inner(music_hoard(vec![]))); @@ -265,6 +300,31 @@ mod tests { assert!(matches!(app, AppState::Match(_))); } + fn lookup_album_expectation(job_sender: &mut MockIMbJobSender, album: &AlbumMeta) { + let requests = VecDeque::from([MbParams::lookup_release_group(album.clone(), mbid())]); + job_sender + .expect_submit_foreground_job() + .with(predicate::always(), predicate::eq(requests)) + .times(1) + .return_once(|_, _| Ok(())); + } + + #[test] + fn lookup_album() { + let mut mb_job_sender = MockIMbJobSender::new(); + + let album = COLLECTION[1].albums[0].meta.clone(); + lookup_album_expectation(&mut mb_job_sender, &album); + + let music_hoard = music_hoard(COLLECTION.to_owned()); + let inner = AppInner::new(music_hoard, mb_job_sender); + + let (_fetch_tx, fetch_rx) = mpsc::channel(); + let fetch = FetchState::new(fetch_rx); + + AppMachine::app_lookup_album(inner, fetch, &album, mbid()); + } + fn search_artist_expectation(job_sender: &mut MockIMbJobSender, artist: &ArtistMeta) { let requests = VecDeque::from([MbParams::search_artist(artist.clone())]); job_sender @@ -294,6 +354,31 @@ mod tests { assert!(matches!(app, AppState::Match(_))); } + fn lookup_artist_expectation(job_sender: &mut MockIMbJobSender, artist: &ArtistMeta) { + let requests = VecDeque::from([MbParams::lookup_artist(artist.clone(), mbid())]); + job_sender + .expect_submit_foreground_job() + .with(predicate::always(), predicate::eq(requests)) + .times(1) + .return_once(|_, _| Ok(())); + } + + #[test] + fn lookup_artist() { + let mut mb_job_sender = MockIMbJobSender::new(); + + let artist = COLLECTION[3].meta.clone(); + lookup_artist_expectation(&mut mb_job_sender, &artist); + + let music_hoard = music_hoard(COLLECTION.to_owned()); + let inner = AppInner::new(music_hoard, mb_job_sender); + + let (_fetch_tx, fetch_rx) = mpsc::channel(); + let fetch = FetchState::new(fetch_rx); + + AppMachine::app_lookup_artist(inner, fetch, &artist, mbid()); + } + #[test] fn fetch_artist_job_sender_err() { let mut mb_job_sender = MockIMbJobSender::new(); @@ -310,6 +395,26 @@ mod tests { assert!(matches!(app, AppState::Error(_))); } + #[test] + fn lookup_artist_job_sender_err() { + let mut mb_job_sender = MockIMbJobSender::new(); + + mb_job_sender + .expect_submit_foreground_job() + .return_once(|_, _| Err(DaemonError::JobChannelDisconnected)); + + let artist = COLLECTION[3].meta.clone(); + + let music_hoard = music_hoard(COLLECTION.to_owned()); + let inner = AppInner::new(music_hoard, mb_job_sender); + + let (_fetch_tx, fetch_rx) = mpsc::channel(); + let fetch = FetchState::new(fetch_rx); + + let app = AppMachine::app_lookup_artist(inner, fetch, &artist, mbid()); + assert!(matches!(app, AppState::Error(_))); + } + #[test] fn recv_ok_fetch_ok() { let (tx, rx) = mpsc::channel::(); diff --git a/src/tui/app/machine/input.rs b/src/tui/app/machine/input.rs index d508f76..32ca237 100644 --- a/src/tui/app/machine/input.rs +++ b/src/tui/app/machine/input.rs @@ -64,20 +64,12 @@ impl IAppInput for AppInputMode { #[cfg(test)] mod tests { use crate::tui::app::{ - machine::tests::{mb_job_sender, music_hoard_init}, + machine::tests::{input_event, mb_job_sender, music_hoard_init}, IApp, }; use super::*; - fn input_event(c: char) -> InputEvent { - crossterm::event::KeyEvent::new( - crossterm::event::KeyCode::Char(c), - crossterm::event::KeyModifiers::empty(), - ) - .into() - } - #[test] fn handle_input() { let mut app = App::new(music_hoard_init(vec![]), mb_job_sender()); diff --git a/src/tui/app/machine/match_state.rs b/src/tui/app/machine/match_state.rs index 5c0a914..2089859 100644 --- a/src/tui/app/machine/match_state.rs +++ b/src/tui/app/machine/match_state.rs @@ -218,8 +218,9 @@ impl IAppInteractMatch for AppMachine { #[cfg(test)] mod tests { - use std::sync::mpsc; + use std::{collections::VecDeque, sync::mpsc}; + use mockall::predicate; use musichoard::collection::{ album::{AlbumDate, AlbumId, AlbumMeta, AlbumPrimaryType, AlbumSecondaryType}, artist::{ArtistId, ArtistMeta}, @@ -227,10 +228,13 @@ mod tests { use crate::tui::{ app::{ - machine::tests::{inner, music_hoard}, + machine::tests::{inner, inner_with_mb, input_event, music_hoard}, IApp, IAppAccess, IAppInput, }, - lib::interface::musicbrainz::api::Match, + lib::interface::musicbrainz::{ + api::{Lookup, Match}, + daemon::{MbParams, MockIMbJobSender}, + }, }; use super::*; @@ -245,6 +249,15 @@ mod tests { } } + impl Lookup { + pub fn new(item: T) -> Self { + Lookup { + item, + disambiguation: None, + } + } + } + fn artist_match() -> MatchStateInfo { let artist = ArtistMeta::new(ArtistId::new("Artist")); @@ -259,6 +272,12 @@ mod tests { MatchStateInfo::artist_search(artist, list) } + fn artist_lookup() -> MatchStateInfo { + let artist = ArtistMeta::new(ArtistId::new("Artist")); + let lookup = Lookup::new(artist.clone()); + MatchStateInfo::artist_lookup(artist, lookup) + } + fn album_match() -> MatchStateInfo { let album = AlbumMeta::new( AlbumId::new("Album"), @@ -279,6 +298,17 @@ mod tests { MatchStateInfo::album_search(album, list) } + fn album_lookup() -> MatchStateInfo { + let album = AlbumMeta::new( + AlbumId::new("Album"), + AlbumDate::new(Some(1990), Some(5), None), + Some(AlbumPrimaryType::Album), + vec![AlbumSecondaryType::Live, AlbumSecondaryType::Compilation], + ); + let lookup = Lookup::new(album.clone()); + MatchStateInfo::album_lookup(album, lookup) + } + fn fetch_state() -> FetchState { let (_, rx) = mpsc::channel(); FetchState::new(rx) @@ -329,7 +359,7 @@ mod tests { assert_eq!(public_matches.state, &widget_state); } - fn match_state_flow(mut matches_info: MatchStateInfo) { + fn match_state_flow(mut matches_info: MatchStateInfo, len: usize) { // tx must exist for rx to return Empty rather than Disconnected. #[allow(unused_variables)] let (tx, rx) = mpsc::channel(); @@ -350,27 +380,30 @@ mod tests { assert_eq!(matches.state.current.as_ref(), Some(&matches_info)); assert_eq!(matches.state.state.list.selected(), Some(0)); - let matches = matches.next_match().unwrap_match(); + let mut matches = matches; + for ii in 1..len { + matches = matches.next_match().unwrap_match(); - assert_eq!(matches.state.current.as_ref(), Some(&matches_info)); - assert_eq!(matches.state.state.list.selected(), Some(1)); + assert_eq!(matches.state.current.as_ref(), Some(&matches_info)); + assert_eq!(matches.state.state.list.selected(), Some(ii)); + } // Next is CannotHaveMBID let matches = matches.next_match().unwrap_match(); assert_eq!(matches.state.current.as_ref(), Some(&matches_info)); - assert_eq!(matches.state.state.list.selected(), Some(2)); + assert_eq!(matches.state.state.list.selected(), Some(len)); // Next is ManualInputMbid let matches = matches.next_match().unwrap_match(); assert_eq!(matches.state.current.as_ref(), Some(&matches_info)); - assert_eq!(matches.state.state.list.selected(), Some(3)); + assert_eq!(matches.state.state.list.selected(), Some(len + 1)); let matches = matches.next_match().unwrap_match(); assert_eq!(matches.state.current.as_ref(), Some(&matches_info)); - assert_eq!(matches.state.state.list.selected(), Some(3)); + assert_eq!(matches.state.state.list.selected(), Some(len + 1)); // Go prev_match first as selecting on manual input does not go back to fetch. let matches = matches.prev_match().unwrap_match(); @@ -379,12 +412,22 @@ mod tests { #[test] fn artist_matches_flow() { - match_state_flow(artist_match()); + match_state_flow(artist_match(), 2); + } + + #[test] + fn artist_lookup_flow() { + match_state_flow(artist_lookup(), 1); } #[test] fn album_matches_flow() { - match_state_flow(album_match()); + match_state_flow(album_match(), 2); + } + + #[test] + fn album_lookup_flow() { + match_state_flow(album_lookup(), 1); } #[test] @@ -430,4 +473,73 @@ mod tests { let input = app.mode().unwrap_input(); input.confirm().unwrap_error(); } + + fn mbid() -> Mbid { + "00000000-0000-0000-0000-000000000000".try_into().unwrap() + } + + fn input_mbid(mut app: App) -> App { + let mbid = mbid().uuid().to_string(); + for c in mbid.chars() { + let input = app.mode().unwrap_input(); + app = input.input(input_event(c)); + } + app + } + + #[test] + fn select_manual_input_artist() { + let mut mb_job_sender = MockIMbJobSender::new(); + let artist = ArtistMeta::new(ArtistId::new("Artist")); + let requests = VecDeque::from([MbParams::lookup_artist(artist.clone(), mbid())]); + mb_job_sender + .expect_submit_foreground_job() + .with(predicate::always(), predicate::eq(requests)) + .return_once(|_, _| Ok(())); + + let matches_vec: Vec> = vec![]; + let artist_match = MatchStateInfo::artist_search(artist.clone(), matches_vec); + let matches = AppMachine::match_state( + inner_with_mb(music_hoard(vec![]), mb_job_sender), + match_state(Some(artist_match)), + ); + + // There are no matches which means that the second option should be manual input. + let matches = matches.next_match().unwrap_match(); + let matches = matches.next_match().unwrap_match(); + + let mut app = matches.select(); + app = input_mbid(app); + + let input = app.mode().unwrap_input(); + input.confirm(); + } + + #[test] + fn select_manual_input_album() { + let mut mb_job_sender = MockIMbJobSender::new(); + let album = AlbumMeta::new::<_, u32>("Album", 1990u32.into(), None, vec![]); + let requests = VecDeque::from([MbParams::lookup_release_group(album.clone(), mbid())]); + mb_job_sender + .expect_submit_foreground_job() + .with(predicate::always(), predicate::eq(requests)) + .return_once(|_, _| Ok(())); + + let matches_vec: Vec> = vec![]; + let album_match = MatchStateInfo::album_search(album.clone(), matches_vec); + let matches = AppMachine::match_state( + inner_with_mb(music_hoard(vec![]), mb_job_sender), + match_state(Some(album_match)), + ); + + // There are no matches which means that the second option should be manual input. + let matches = matches.next_match().unwrap_match(); + let matches = matches.next_match().unwrap_match(); + + let mut app = matches.select(); + app = input_mbid(app); + + let input = app.mode().unwrap_input(); + input.confirm(); + } } diff --git a/src/tui/app/machine/mod.rs b/src/tui/app/machine/mod.rs index 35b6542..87dfea8 100644 --- a/src/tui/app/machine/mod.rs +++ b/src/tui/app/machine/mod.rs @@ -222,7 +222,7 @@ mod tests { use musichoard::collection::Collection; use crate::tui::{ - app::{AppState, IApp, IAppInput, IAppInteractBrowse}, + app::{AppState, IApp, IAppInput, IAppInteractBrowse, InputEvent}, lib::{interface::musicbrainz::daemon::MockIMbJobSender, MockIMusicHoard}, }; @@ -355,6 +355,14 @@ mod tests { AppInner::new(music_hoard, mb_job_sender) } + pub fn input_event(c: char) -> InputEvent { + crossterm::event::KeyEvent::new( + crossterm::event::KeyCode::Char(c), + crossterm::event::KeyModifiers::empty(), + ) + .into() + } + #[test] fn input_mode() { let app = App::new(music_hoard_init(vec![]), mb_job_sender()); diff --git a/src/tui/lib/external/musicbrainz/daemon/mod.rs b/src/tui/lib/external/musicbrainz/daemon/mod.rs index 69fbdfa..8d0f9e9 100644 --- a/src/tui/lib/external/musicbrainz/daemon/mod.rs +++ b/src/tui/lib/external/musicbrainz/daemon/mod.rs @@ -110,7 +110,7 @@ impl IMbJobSender for JobSender { result_sender: ResultSender, requests: VecDeque, ) -> Result<(), Error> { - self.send_foreground_job(result_sender, requests) + self.send_job(JobPriority::Foreground, result_sender, requests) } fn submit_background_job( @@ -118,27 +118,11 @@ impl IMbJobSender for JobSender { result_sender: ResultSender, requests: VecDeque, ) -> Result<(), Error> { - self.send_background_job(result_sender, requests) + self.send_job(JobPriority::Background, result_sender, requests) } } impl JobSender { - fn send_foreground_job( - &self, - result_sender: ResultSender, - requests: VecDeque, - ) -> Result<(), Error> { - self.send_job(JobPriority::Foreground, result_sender, requests) - } - - fn send_background_job( - &self, - result_sender: ResultSender, - requests: VecDeque, - ) -> Result<(), Error> { - self.send_job(JobPriority::Background, result_sender, requests) - } - fn send_job( &self, priority: JobPriority, @@ -337,7 +321,7 @@ mod tests { use crate::tui::{ event::{Event, EventError, MockIFetchCompleteEventSender}, - lib::interface::musicbrainz::api::{Match, MockIMusicBrainz}, + lib::interface::musicbrainz::api::{Lookup, Match, MockIMusicBrainz}, testmod::COLLECTION, }; @@ -387,12 +371,28 @@ mod tests { } } + fn mbid() -> Mbid { + "00000000-0000-0000-0000-000000000000".try_into().unwrap() + } + + fn lookup_artist_requests() -> VecDeque { + let artist = COLLECTION[3].meta.clone(); + let mbid = mbid(); + VecDeque::from([MbParams::lookup_artist(artist, mbid)]) + } + + fn lookup_release_group_requests() -> VecDeque { + let album = COLLECTION[1].albums[0].meta.clone(); + let mbid = mbid(); + VecDeque::from([MbParams::lookup_release_group(album, mbid)]) + } + fn search_artist_requests() -> VecDeque { let artist = COLLECTION[3].meta.clone(); VecDeque::from([MbParams::search_artist(artist)]) } - fn artist_expectations() -> (ArtistMeta, Vec>) { + fn search_artist_expectations() -> (ArtistMeta, Vec>) { let artist = COLLECTION[3].meta.clone(); let artist_match_1 = Match::new(100, artist.clone()); @@ -420,7 +420,7 @@ mod tests { mbref.unwrap().mbid().clone() } - fn album_expectations_1() -> (AlbumMeta, Vec>) { + fn search_album_expectations_1() -> (AlbumMeta, Vec>) { let album_1 = COLLECTION[1].albums[0].meta.clone(); let album_4 = COLLECTION[1].albums[3].meta.clone(); @@ -431,7 +431,7 @@ mod tests { (album_1, matches_1) } - fn album_expectations_4() -> (AlbumMeta, Vec>) { + fn search_album_expectations_4() -> (AlbumMeta, Vec>) { let album_1 = COLLECTION[1].albums[0].meta.clone(); let album_4 = COLLECTION[1].albums[3].meta.clone(); @@ -515,6 +515,90 @@ mod tests { assert_eq!(result, Err(JobError::JobQueueEmpty)); } + fn lookup_artist_expectation( + musicbrainz: &mut MockIMusicBrainz, + mbid: &Mbid, + lookup: &Lookup, + ) { + let result = Ok(lookup.clone()); + musicbrainz + .expect_lookup_artist() + .with(predicate::eq(mbid.clone())) + .times(1) + .return_once(|_| result); + } + + #[test] + fn execute_lookup_artist() { + let mut musicbrainz = musicbrainz(); + let mbid = mbid(); + let artist = COLLECTION[3].meta.clone(); + let lookup = Lookup::new(artist.clone()); + lookup_artist_expectation(&mut musicbrainz, &mbid, &lookup); + + let mut event_sender = event_sender(); + fetch_complete_expectation(&mut event_sender, 1); + + let (job_sender, job_receiver) = job_channel(); + let mut daemon = daemon_with(musicbrainz, job_receiver, event_sender); + + let requests = lookup_artist_requests(); + let (result_sender, result_receiver) = mpsc::channel(); + let result = job_sender.submit_foreground_job(result_sender, requests); + assert_eq!(result, Ok(())); + + let result = daemon.enqueue_all_pending_jobs(); + assert_eq!(result, Ok(())); + + let result = daemon.execute_next_job(); + assert_eq!(result, Ok(())); + + let result = result_receiver.try_recv().unwrap(); + assert_eq!(result, Ok(MatchStateInfo::artist_lookup(artist, lookup))); + } + + fn lookup_release_group_expectation( + musicbrainz: &mut MockIMusicBrainz, + mbid: &Mbid, + lookup: &Lookup, + ) { + let result = Ok(lookup.clone()); + musicbrainz + .expect_lookup_release_group() + .with(predicate::eq(mbid.clone())) + .times(1) + .return_once(|_| result); + } + + #[test] + fn execute_lookup_release_group() { + let mut musicbrainz = musicbrainz(); + let mbid = mbid(); + let album = COLLECTION[1].albums[0].meta.clone(); + let lookup = Lookup::new(album.clone()); + lookup_release_group_expectation(&mut musicbrainz, &mbid, &lookup); + + let mut event_sender = event_sender(); + fetch_complete_expectation(&mut event_sender, 1); + + let (job_sender, job_receiver) = job_channel(); + let mut daemon = daemon_with(musicbrainz, job_receiver, event_sender); + + let requests = lookup_release_group_requests(); + let (result_sender, result_receiver) = mpsc::channel(); + let result = job_sender.submit_foreground_job(result_sender, requests); + assert_eq!(result, Ok(())); + + let result = daemon.enqueue_all_pending_jobs(); + assert_eq!(result, Ok(())); + + let result = daemon.execute_next_job(); + assert_eq!(result, Ok(())); + + let result = result_receiver.try_recv().unwrap(); + assert_eq!(result, Ok(MatchStateInfo::album_lookup(album, lookup))); + } + fn search_artist_expectation( musicbrainz: &mut MockIMusicBrainz, artist: &ArtistMeta, @@ -531,7 +615,7 @@ mod tests { #[test] fn execute_search_artist() { let mut musicbrainz = musicbrainz(); - let (artist, matches) = artist_expectations(); + let (artist, matches) = search_artist_expectations(); search_artist_expectation(&mut musicbrainz, &artist, &matches); let mut event_sender = event_sender(); @@ -575,8 +659,8 @@ mod tests { fn execute_search_release_groups() { let mut musicbrainz = musicbrainz(); let arid = album_arid_expectation(); - let (album_1, matches_1) = album_expectations_1(); - let (album_4, matches_4) = album_expectations_4(); + let (album_1, matches_1) = search_album_expectations_1(); + let (album_4, matches_4) = search_album_expectations_4(); let mut seq = Sequence::new(); search_release_group_expectation(&mut musicbrainz, &mut seq, &arid, &album_1, &matches_1); @@ -613,7 +697,7 @@ mod tests { fn execute_search_release_groups_result_disconnect() { let mut musicbrainz = musicbrainz(); let arid = album_arid_expectation(); - let (album_1, matches_1) = album_expectations_1(); + let (album_1, matches_1) = search_album_expectations_1(); let mut seq = Sequence::new(); search_release_group_expectation(&mut musicbrainz, &mut seq, &arid, &album_1, &matches_1); @@ -643,7 +727,7 @@ mod tests { #[test] fn execute_search_artist_event_disconnect() { let mut musicbrainz = musicbrainz(); - let (artist, matches) = artist_expectations(); + let (artist, matches) = search_artist_expectations(); search_artist_expectation(&mut musicbrainz, &artist, &matches); let mut event_sender = event_sender();