diff --git a/Cargo.lock b/Cargo.lock index 7085a01..2c4ba45 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -326,6 +326,16 @@ dependencies = [ "winapi", ] +[[package]] +name = "ctor" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a2785755761f3ddc1492979ce1e48d2c00d09311c39e4466429188f3dd6501" +dependencies = [ + "quote", + "syn", +] + [[package]] name = "darling" version = "0.20.11" @@ -772,9 +782,11 @@ dependencies = [ "chrono", "clap", "color-eyre", + "ctor", "derive-getters", "lazy_static", "mockall", + "once_cell", "patch-hub-proc-macros 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "ratatui", "regex", diff --git a/Cargo.toml b/Cargo.toml index d498a1f..d98e137 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,6 +25,10 @@ ansi-to-tui = "7.0.0" which = "8.0.0" ureq = { version = "3.0.12", features = ["rustls"] } +[dev-dependencies] +ctor = "0.2" +once_cell = "1.19" + # The profile that 'cargo dist' will build with [profile.dist] inherits = "release" diff --git a/src/app/mod.rs b/src/app/mod.rs index 674e677..dca991c 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -120,7 +120,7 @@ impl App { self.latest_patchsets = Some(LatestPatchsets::new( target_list, self.config.page_size(), - self.lore_api_client.clone(), + Box::new(self.lore_api_client.clone()), )); } diff --git a/src/app/screens/latest.rs b/src/app/screens/latest.rs index ce7d448..76cfe47 100644 --- a/src/app/screens/latest.rs +++ b/src/app/screens/latest.rs @@ -2,7 +2,7 @@ use color_eyre::eyre::bail; use derive_getters::Getters; use crate::lore::{ - lore_api_client::{BlockingLoreAPIClient, ClientError}, + lore_api_client::{ClientError, PatchFeedRequest}, lore_session::{LoreSession, LoreSessionError}, patch::Patch, }; @@ -10,7 +10,7 @@ use crate::lore::{ #[derive(Getters)] pub struct LatestPatchsets { lore_session: LoreSession, - lore_api_client: BlockingLoreAPIClient, + lore_api_client: Box, target_list: String, page_number: usize, patchset_index: usize, @@ -21,7 +21,7 @@ impl LatestPatchsets { pub fn new( target_list: String, page_size: usize, - lore_api_client: BlockingLoreAPIClient, + lore_api_client: Box, ) -> LatestPatchsets { LatestPatchsets { lore_session: LoreSession::new(target_list.clone()), @@ -35,7 +35,7 @@ impl LatestPatchsets { pub fn fetch_current_page(&mut self) -> color_eyre::Result<()> { if let Err(lore_session_error) = self.lore_session.process_n_representative_patches( - &self.lore_api_client, + self.lore_api_client.as_ref(), self.page_size * self.page_number, ) { match lore_session_error { @@ -106,3 +106,437 @@ impl LatestPatchsets { self.lore_session.representative_patches_ids().len() } } + +#[cfg(test)] +mod tests { + use crate::lore::lore_api_client::MockPatchFeedRequest; + use std::fs; + + use super::*; + + #[test] + fn test_fetch_current_page_success() { + let src_path = + "test_samples/lore_session/process_representative_patch/patch_feed_sample_1.xml"; + let target_list = "some-list"; + + let mut lore_api_client = MockPatchFeedRequest::new(); + + lore_api_client + .expect_request_patch_feed() + .withf(move |target_list_arg, min_index_arg| { + target_list_arg == target_list && *min_index_arg == 0 + }) + .times(1) + .returning(move |_, _| Ok(fs::read_to_string(src_path).unwrap())); + + let mut latest_patchsets = + LatestPatchsets::new(target_list.to_string(), 0, Box::new(lore_api_client)); + latest_patchsets.page_size = 1; + latest_patchsets.page_number = 1; + + let fetch_result = latest_patchsets.fetch_current_page(); + + assert!(fetch_result.is_ok()); + assert_eq!(latest_patchsets.page_size, 1); + assert_eq!(latest_patchsets.page_number, 1); + assert_eq!(latest_patchsets.patchset_index, 0); + + assert_eq!(latest_patchsets.processed_patchsets_count(), 1); + } + + #[test] + fn test_fetch_current_page_end_of_feed() { + let mut lore_api_client = MockPatchFeedRequest::new(); + let target_list = "some-list"; + + lore_api_client + .expect_request_patch_feed() + .withf(move |target_list_arg, min_index_arg| { + target_list_arg == target_list && *min_index_arg == 0 + }) + .times(1) + .returning(move |_, _| Err(ClientError::EndOfFeed)); + + let mut latest_patchsets = + LatestPatchsets::new(target_list.to_string(), 0, Box::new(lore_api_client)); + latest_patchsets.page_size = 1; + latest_patchsets.page_number = 1; + + assert_eq!(latest_patchsets.patchset_index, 0); + assert_eq!(latest_patchsets.processed_patchsets_count(), 0); + + let fetch_result = latest_patchsets.fetch_current_page(); + + assert!(fetch_result.is_ok()); + assert_eq!(latest_patchsets.patchset_index, 0); + + assert_eq!(latest_patchsets.processed_patchsets_count(), 0); + } + + #[test] + fn test_fetch_current_page_client_error() { + let mut lore_api_client = MockPatchFeedRequest::new(); + let target_list = "some-list"; + + lore_api_client + .expect_request_patch_feed() + .withf(move |target_list_arg, min_index_arg| { + target_list_arg == target_list && *min_index_arg == 0 + }) + .times(1) + .returning(move |_, _| Err(ClientError::FromUreq(ureq::Error::StatusCode(401)))); + + let mut latest_patchsets = + LatestPatchsets::new(target_list.to_string(), 0, Box::new(lore_api_client)); + latest_patchsets.page_size = 1; + latest_patchsets.page_number = 1; + + let fetch_result = latest_patchsets.fetch_current_page(); + + assert!(fetch_result.is_err()); + assert_eq!(latest_patchsets.patchset_index, 0); + + assert_eq!(latest_patchsets.processed_patchsets_count(), 0); + } + + #[test] + fn test_select_below_patchset() { + // initializing LatestPatchsets so we can test select_below_patchset properly + let mut latest_patchsets = { + let target_list = "some-list"; + let page_size = 3; + + let src_path = + "test_samples/lore_session/process_representative_patch/patch_feed_sample_2.xml"; + + let mut lore_api_client = MockPatchFeedRequest::new(); + + let target_list_string = target_list.to_string(); + lore_api_client + .expect_request_patch_feed() + .withf(move |target_list_arg, min_index_arg| { + target_list_arg == target_list_string && *min_index_arg == 0 + }) + .times(1) + .returning(move |_, _| Ok(fs::read_to_string(src_path).unwrap())); + LatestPatchsets::new( + target_list.to_string(), + page_size, + Box::new(lore_api_client), + ) + }; + + // asserting we have patchsets to read + latest_patchsets.fetch_current_page().expect("to fetch"); + assert_eq!(latest_patchsets.processed_patchsets_count(), 3); + + // test case 1: base case + latest_patchsets.patchset_index = 0; + latest_patchsets.page_size = 0; + latest_patchsets.page_number = 1; + + latest_patchsets.select_below_patchset(); + assert_eq!(latest_patchsets.patchset_index(), 0); + + // test case 2: selecting below patchset is possible + latest_patchsets.patchset_index = 0; + latest_patchsets.page_size = 2; + latest_patchsets.page_number = 1; + + latest_patchsets.select_below_patchset(); + assert_eq!(latest_patchsets.patchset_index(), 1); + + // test case 3: already on the bottom of the page + latest_patchsets.patchset_index = 1; + latest_patchsets.page_size = 2; + latest_patchsets.page_number = 1; + + latest_patchsets.select_below_patchset(); + assert_eq!(latest_patchsets.patchset_index(), 1); + + // test case 4: incrementing page so we can select below patchset + latest_patchsets.patchset_index = 1; + latest_patchsets.page_size = 2; + latest_patchsets.page_number = 2; + + latest_patchsets.select_below_patchset(); + assert_eq!(latest_patchsets.patchset_index(), 2); + } + + #[test] + fn test_select_above_patchset() { + let mut latest_patchsets = + LatestPatchsets::new("".to_string(), 0, Box::new(MockPatchFeedRequest::new())); + + // test case 1: base case + latest_patchsets.patchset_index = 0; + latest_patchsets.page_size = 0; + latest_patchsets.page_number = 1; + + latest_patchsets.select_above_patchset(); + assert_eq!(latest_patchsets.patchset_index(), 0); + + // test case 2: patchset 0 is always the topmost + latest_patchsets.patchset_index = 0; + latest_patchsets.page_size = 10; + latest_patchsets.page_number = 2; + + latest_patchsets.select_above_patchset(); + assert_eq!(latest_patchsets.patchset_index(), 0); + + // test case 3: current patchset is already the top of the page + latest_patchsets.patchset_index = 2; + latest_patchsets.page_size = 2; + latest_patchsets.page_number = 2; + + latest_patchsets.select_above_patchset(); + assert_eq!(latest_patchsets.patchset_index(), 2); + + // test case 4: selecting above until the end of the page + latest_patchsets.patchset_index = 24; + latest_patchsets.page_size = 5; + latest_patchsets.page_number = 5; + + latest_patchsets.select_above_patchset(); + assert_eq!(latest_patchsets.patchset_index(), 23); + + latest_patchsets.select_above_patchset(); + assert_eq!(latest_patchsets.patchset_index(), 22); + + latest_patchsets.select_above_patchset(); + assert_eq!(latest_patchsets.patchset_index(), 21); + + latest_patchsets.select_above_patchset(); + assert_eq!(latest_patchsets.patchset_index(), 20); + + latest_patchsets.select_above_patchset(); + assert_eq!(latest_patchsets.patchset_index(), 20); + } + + #[test] + fn test_increment_page() { + // initializing LatestPatchsets so we can test increment_page properly + let mut latest_patchsets = { + let target_list = "some-list"; + let page_size = 3; + + let src_path = + "test_samples/lore_session/process_representative_patch/patch_feed_sample_2.xml"; + + let mut lore_api_client = MockPatchFeedRequest::new(); + + let target_list_string = target_list.to_string(); + lore_api_client + .expect_request_patch_feed() + .withf(move |target_list_arg, min_index_arg| { + target_list_arg == target_list_string && *min_index_arg == 0 + }) + .times(1) + .returning(move |_, _| Ok(fs::read_to_string(src_path).unwrap())); + LatestPatchsets::new( + target_list.to_string(), + page_size, + Box::new(lore_api_client), + ) + }; + + // asserting we have patchsets to read + latest_patchsets.fetch_current_page().expect("to fetch"); + assert_eq!(latest_patchsets.processed_patchsets_count(), 3); + + // test case 1: success incrementing page + latest_patchsets.patchset_index = 0; + latest_patchsets.page_size = 1; + latest_patchsets.page_number = 1; + + latest_patchsets.increment_page(); + assert_eq!(latest_patchsets.patchset_index, 1); + assert_eq!(latest_patchsets.page_size, 1); + assert_eq!(latest_patchsets.page_number, 2); + + // test case 3: patchset index is overwritten when page is incremented + latest_patchsets.patchset_index = 999; + latest_patchsets.page_size = 1; + latest_patchsets.page_number = 1; + + latest_patchsets.increment_page(); + assert_eq!(latest_patchsets.patchset_index, 1); + assert_eq!(latest_patchsets.page_size, 1); + assert_eq!(latest_patchsets.page_number, 2); + + // test case 4: won't increment after max page + latest_patchsets.patchset_index = 0; + latest_patchsets.page_size = 10; + latest_patchsets.page_number = 1; + + latest_patchsets.increment_page(); + assert_eq!(latest_patchsets.patchset_index, 0); + assert_eq!(latest_patchsets.page_size, 10); + assert_eq!(latest_patchsets.page_number, 1); + + // test case 5: sequencial increments + latest_patchsets.patchset_index = 0; + latest_patchsets.page_size = 1; + latest_patchsets.page_number = 1; + + latest_patchsets.increment_page(); + assert_eq!(latest_patchsets.patchset_index, 1); + assert_eq!(latest_patchsets.page_size, 1); + assert_eq!(latest_patchsets.page_number, 2); + + latest_patchsets.increment_page(); + assert_eq!(latest_patchsets.patchset_index, 2); + assert_eq!(latest_patchsets.page_size, 1); + assert_eq!(latest_patchsets.page_number, 3); + + latest_patchsets.increment_page(); + assert_eq!(latest_patchsets.patchset_index, 3); + assert_eq!(latest_patchsets.page_size, 1); + assert_eq!(latest_patchsets.page_number, 4); + + latest_patchsets.increment_page(); + assert_eq!(latest_patchsets.patchset_index, 3); + assert_eq!(latest_patchsets.page_size, 1); + assert_eq!(latest_patchsets.page_number, 4); + } + + #[test] + fn test_decrement_page() { + let mut latest_patchsets = + LatestPatchsets::new("".to_string(), 0, Box::new(MockPatchFeedRequest::new())); + + // test case 1: already in the first page + latest_patchsets.page_number = 1; + latest_patchsets.page_size = 0; + + latest_patchsets.decrement_page(); + assert_eq!(latest_patchsets.page_number(), 1); + assert_eq!(latest_patchsets.patchset_index(), 0); + + // test case 2: second page + latest_patchsets.page_number = 2; + latest_patchsets.page_size = 3; + latest_patchsets.patchset_index = 9; // this doesn't matter, will be overwritten + + latest_patchsets.decrement_page(); + assert_eq!(latest_patchsets.page_number(), 1); + assert_eq!(latest_patchsets.patchset_index(), 0); + + // test case 3: decrementing page until the first + latest_patchsets.page_number = 5; + latest_patchsets.page_size = 100; + + latest_patchsets.decrement_page(); + assert_eq!(latest_patchsets.page_number(), 4); + assert_eq!(latest_patchsets.patchset_index(), 300); + + latest_patchsets.decrement_page(); + assert_eq!(latest_patchsets.page_number(), 3); + assert_eq!(latest_patchsets.patchset_index(), 200); + + latest_patchsets.decrement_page(); + assert_eq!(latest_patchsets.page_number(), 2); + assert_eq!(latest_patchsets.patchset_index(), 100); + + latest_patchsets.decrement_page(); + assert_eq!(latest_patchsets.page_number(), 1); + assert_eq!(latest_patchsets.patchset_index(), 0); + + latest_patchsets.decrement_page(); + assert_eq!(latest_patchsets.page_number(), 1); + assert_eq!(latest_patchsets.patchset_index(), 0); + } + + #[test] + #[should_panic] + fn test_get_selected_patchset_before_fetching_page() { + let latest_patchsets = + LatestPatchsets::new("".to_string(), 3, Box::new(MockPatchFeedRequest::new())); + + let _patch = latest_patchsets.get_selected_patchset(); + } + + #[test] + fn test_get_selected_patchset() { + // initializing LatestPatchsets so we can test get_selected_patchset properly + let mut latest_patchsets = { + let target_list = "some-list"; + let page_size = 3; + + let src_path = + "test_samples/lore_session/process_representative_patch/patch_feed_sample_2.xml"; + + let mut lore_api_client = MockPatchFeedRequest::new(); + + let target_list_string = target_list.to_string(); + lore_api_client + .expect_request_patch_feed() + .withf(move |target_list_arg, min_index_arg| { + target_list_arg == target_list_string && *min_index_arg == 0 + }) + .times(1) + .returning(move |_, _| Ok(fs::read_to_string(src_path).unwrap())); + LatestPatchsets::new( + target_list.to_string(), + page_size, + Box::new(lore_api_client), + ) + }; + // asserting we have patchsets to read + latest_patchsets.fetch_current_page().expect("to fetch"); + assert_eq!(latest_patchsets.processed_patchsets_count(), 3); + + latest_patchsets.patchset_index = 0; + let patch = latest_patchsets.get_selected_patchset(); + assert!(patch + .message_id() + .href + .contains("1234.567-1-roberto@silva.br")); + + latest_patchsets.patchset_index = 1; + let patch = latest_patchsets.get_selected_patchset(); + assert!(patch.message_id().href.contains("first-patch-lima@luma.rs")); + + latest_patchsets.patchset_index = 2; + let patch = latest_patchsets.get_selected_patchset(); + assert!(patch + .message_id() + .href + .contains("1234.567-1-john@johnson.com")); + } + + #[test] + #[should_panic] + fn test_get_selected_patchset_invalid_index() { + let mut latest_patchsets = { + let target_list = "some-list"; + let page_size = 3; + + let src_path = + "test_samples/lore_session/process_representative_patch/patch_feed_sample_2.xml"; + + let mut lore_api_client = MockPatchFeedRequest::new(); + + let target_list_string = target_list.to_string(); + lore_api_client + .expect_request_patch_feed() + .withf(move |target_list_arg, min_index_arg| { + target_list_arg == target_list_string && *min_index_arg == 0 + }) + .times(1) + .returning(move |_, _| Ok(fs::read_to_string(src_path).unwrap())); + LatestPatchsets::new( + target_list.to_string(), + page_size, + Box::new(lore_api_client), + ) + }; + // asserting we have patchsets to read + latest_patchsets.fetch_current_page().expect("to fetch"); + assert_eq!(latest_patchsets.processed_patchsets_count(), 3); + + latest_patchsets.patchset_index = 999; + let _patch = latest_patchsets.get_selected_patchset(); + } +} diff --git a/src/infrastructure/errors.rs b/src/infrastructure/errors.rs index a633527..19e9d70 100644 --- a/src/infrastructure/errors.rs +++ b/src/infrastructure/errors.rs @@ -29,28 +29,10 @@ pub fn install_hooks() -> color_eyre::Result<()> { #[cfg(test)] mod tests { - use std::sync::Once; - use super::*; - static INIT: Once = Once::new(); - - // Tests can be run in parallel, we don't want to override previously installed hooks - fn setup() { - INIT.call_once(|| { - install_hooks().expect("Failed to install hooks"); - }) - } - - #[test] - fn test_install_hooks() { - setup(); - } - #[test] fn test_error_hook_works() { - setup(); - let result: color_eyre::Result<()> = Err(color_eyre::eyre::eyre!("Test error")); // We can't directly test the hook's formatting, but we can verify @@ -65,8 +47,6 @@ mod tests { #[test] fn test_panic_hook() { - setup(); - let result = std::panic::catch_unwind(|| std::panic!("Test panic")); assert!(result.is_err()); diff --git a/src/lore/lore_session/mod.rs b/src/lore/lore_session/mod.rs index a073a38..2284cb6 100644 --- a/src/lore/lore_session/mod.rs +++ b/src/lore/lore_session/mod.rs @@ -66,9 +66,9 @@ impl LoreSession { self.processed_patches_map.get(message_id) } - pub fn process_n_representative_patches( + pub fn process_n_representative_patches( &mut self, - lore_api_client: &T, + lore_api_client: &dyn PatchFeedRequest, n: usize, ) -> Result<(), LoreSessionError> { while self.representative_patches_ids.len() < n { diff --git a/src/lore/lore_session/tests.rs b/src/lore/lore_session/tests.rs index 141692d..6e294e0 100644 --- a/src/lore/lore_session/tests.rs +++ b/src/lore/lore_session/tests.rs @@ -1,7 +1,10 @@ use io::Read; use std::fs; -use crate::lore::{lore_api_client::MockBlockingLoreAPIClient, patch::Author}; +use crate::lore::{ + lore_api_client::{MockBlockingLoreAPIClient, MockPatchFeedRequest}, + patch::Author, +}; use super::*; @@ -20,7 +23,7 @@ fn should_process_one_representative_patch() { let src_path = "test_samples/lore_session/process_representative_patch/patch_feed_sample_1.xml"; let target_list = "some-list"; - let mut lore_api_client = MockBlockingLoreAPIClient::new(); + let mut lore_api_client = MockPatchFeedRequest::new(); lore_api_client .expect_request_patch_feed() @@ -84,7 +87,7 @@ fn should_process_multiple_representative_patches() { let src_path = "test_samples/lore_session/process_representative_patch/patch_feed_sample_2.xml"; let target_list = "some-list"; - let mut lore_api_client = MockBlockingLoreAPIClient::new(); + let mut lore_api_client = MockPatchFeedRequest::new(); lore_api_client .expect_request_patch_feed() diff --git a/src/main.rs b/src/main.rs index 1db1440..a871bae 100644 --- a/src/main.rs +++ b/src/main.rs @@ -45,3 +45,22 @@ fn main() -> color_eyre::Result<()> { Ok(()) } + +/// This setup must be done so no test can install the default hooks (when reaching a bail!, for example) +/// before we setup our custom hooks. +#[cfg(test)] +mod test_setup { + use once_cell::sync::Lazy; + + use crate::infrastructure::errors::install_hooks; + + static INIT_HOOKS: Lazy<()> = Lazy::new(|| { + install_hooks().expect("Failed to install hooks"); + }); + + // This will run before any other test and assure the hooks are installed once + #[ctor::ctor] + fn init() { + Lazy::force(&INIT_HOOKS); + } +}