From c571e9ee8ddad8333889846b59b612461248136f Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sun, 30 Jun 2024 07:41:13 +0100 Subject: [PATCH] refactor: CloneRepo use actor::do_send to send LoadConfigFromRepo --- crates/repo-actor/README.md | 23 ++++++----- crates/repo-actor/src/handlers/clone_repo.rs | 22 +++++----- crates/repo-actor/src/tests/expect.rs | 7 ---- crates/repo-actor/src/tests/given.rs | 40 ------------------- .../src/tests/handlers/clone_repo.rs | 35 ++++------------ crates/repo-actor/src/tests/mod.rs | 1 - 6 files changed, 31 insertions(+), 97 deletions(-) diff --git a/crates/repo-actor/README.md b/crates/repo-actor/README.md index bdb4fe0..7bb3579 100644 --- a/crates/repo-actor/README.md +++ b/crates/repo-actor/README.md @@ -1,21 +1,22 @@ ```mermaid stateDiagram-v2 -[*] --> CloneRepo :Start + CloneRepo :Start --> -CloneRepo --> LoadConfigFromRepo -CloneRepo --> ValidateRepo +CloneRepo --> LoadConfigFromRepo :on repo config +CloneRepo --> ValidateRepo :on server config LoadConfigFromRepo --> ReceiveRepoConfig -ValidateRepo --> CheckCIStatus -ValidateRepo --> AdvanceNext -ValidateRepo --> ValidateRepo :invalid +ValidateRepo --> CheckCIStatus :on next ahead of main +ValidateRepo --> AdvanceNext :on dev ahead of next +ValidateRepo --> [*] :on dev == next == main +ValidateRepo --> ValidateRepo :on invalid CheckCIStatus --> ReceiveCIStatus -ReceiveCIStatus --> AdvanceMain :Pass -ReceiveCIStatus --> ValidateRepo :Pending -ReceiveCIStatus --> [*] :Fail +ReceiveCIStatus --> AdvanceMain :on Pass +ReceiveCIStatus --> ValidateRepo :on Pending +ReceiveCIStatus --> [*] :on Fail AdvanceNext --> ValidateRepo @@ -26,10 +27,10 @@ RegisterWebhook --> WebhookRegistered WebhookRegistered --> [*] -AdvanceMain --> LoadConfigFromRepo :on repo config - reload +AdvanceMain --> LoadConfigFromRepo :on repo config AdvanceMain --> ValidateRepo :on server config -[*] --> WebhookNotification :WEBHOOK +[*] --> WebhookNotification :on push WebhookNotification --> ValidateRepo ``` diff --git a/crates/repo-actor/src/handlers/clone_repo.rs b/crates/repo-actor/src/handlers/clone_repo.rs index f474517..752300e 100644 --- a/crates/repo-actor/src/handlers/clone_repo.rs +++ b/crates/repo-actor/src/handlers/clone_repo.rs @@ -12,6 +12,7 @@ impl Handler for actor::RepoActor { _msg: actor::messages::CloneRepo, ctx: &mut Self::Context, ) -> Self::Result { + actor::logger(&self.log, "Handler: CloneRepo: start"); tracing::debug!("Handler: CloneRepo: start"); let gitdir = self.repo_details.gitdir.clone(); match git::repository::open(&*self.repository_factory, &self.repo_details, gitdir) { @@ -20,18 +21,17 @@ impl Handler for actor::RepoActor { tracing::debug!("open okay"); self.open_repository.replace(repository); if self.repo_details.repo_config.is_none() { - tracing::debug!("Handler: CloneRepo: Sending: LoadConfigFromRepo"); - actor::logger(&self.log, "send: LoadConfigFromRepo"); - ctx.address().do_send(actor::messages::LoadConfigFromRepo); + actor::do_send( + ctx.address(), + actor::messages::LoadConfigFromRepo, + &self.log, + ); } else { - tracing::debug!("Handler: CloneRepo: Sending: ValidateRepo"); - actor::logger(&self.log, "send: ValidateRepo"); - if let Err(_e) = ctx - .address() - .try_send(actor::messages::ValidateRepo::new(self.message_token)) - { - actor::logger(&self.log, format!("ValidateRepo: error: {_e:?}")); - } + actor::do_send( + ctx.address(), + actor::messages::ValidateRepo::new(self.message_token), + &self.log, + ); } } Err(err) => { diff --git a/crates/repo-actor/src/tests/expect.rs b/crates/repo-actor/src/tests/expect.rs index 8472f4d..018c135 100644 --- a/crates/repo-actor/src/tests/expect.rs +++ b/crates/repo-actor/src/tests/expect.rs @@ -26,13 +26,6 @@ pub fn push(open_repository: &mut MockOpenRepositoryLike, result: Result<(), git .return_once(move |_, _, _, _| result); } -pub fn duplicate(open_repository: &mut MockOpenRepositoryLike, result: MockOpenRepositoryLike) { - open_repository - .expect_duplicate() - .times(1) - .return_once(move || Box::new(result)); -} - pub fn open_repository( repository_factory: &mut MockRepositoryFactory, open_repository: MockOpenRepositoryLike, diff --git a/crates/repo-actor/src/tests/given.rs b/crates/repo-actor/src/tests/given.rs index 96edd9a..f47583a 100644 --- a/crates/repo-actor/src/tests/given.rs +++ b/crates/repo-actor/src/tests/given.rs @@ -26,46 +26,6 @@ pub fn has_remote_defaults( }); } -#[allow(clippy::type_complexity)] -pub fn open_repository_for_loading_config_from_repo( - repo_config: &RepoConfig, -) -> ( - MockOpenRepositoryLike, - Arc>>, -) { - let mut load_config_from_repo_open_repository = MockOpenRepositoryLike::new(); - let read_files = Arc::new(Mutex::new(vec![])); - let read_files_ref = read_files.clone(); - let branches = repo_config.branches().clone(); - load_config_from_repo_open_repository - .expect_read_file() - .return_once(move |branch_name, file_name| { - let branch_name = branch_name.clone(); - let file_name = file_name.to_path_buf(); - let _ = read_files_ref - .lock() - .map(move |mut l| l.push((branch_name, file_name))); - let contents = format!( - r#" - [branches] - main = "{}" - next = "{}" - dev = "{}" - "#, - branches.main(), - branches.next(), - branches.dev() - ); - Ok(contents) - }); - let branches = repo_config.branches().clone(); - let remote_branches = vec![branches.main(), branches.next(), branches.dev()]; - load_config_from_repo_open_repository - .expect_remote_branches() - .return_once(move || Ok(remote_branches)); - (load_config_from_repo_open_repository, read_files) -} - pub fn a_webhook_auth() -> WebhookAuth { WebhookAuth::generate() } diff --git a/crates/repo-actor/src/tests/handlers/clone_repo.rs b/crates/repo-actor/src/tests/handlers/clone_repo.rs index 81495b7..6d80dc9 100644 --- a/crates/repo-actor/src/tests/handlers/clone_repo.rs +++ b/crates/repo-actor/src/tests/handlers/clone_repo.rs @@ -78,42 +78,29 @@ async fn should_open() -> TestResult { } /// The server config can optionally include the names of the main, next and dev -/// branches. When it doesn't we should load the .git-next.yaml from from the -/// repo and get the branch names from there. +/// branches. When it doesn't we should load the `.git-next.yaml` from from the +/// repo and get the branch names from there by sending a [LoadConfigFromRepo] message. #[actix::test] -async fn when_server_has_no_repo_config_load_from_repo_and_validate() -> TestResult { +async fn when_server_has_no_repo_config_should_send_load_from_repo() -> TestResult { //given let fs = given::a_filesystem(); let (mut open_repository, mut repo_details) = given::an_open_repository(&fs); #[allow(clippy::unwrap_used)] - let repo_config = repo_details.repo_config.take().unwrap(); + let _repo_config = repo_details.repo_config.take().unwrap(); given::has_all_valid_remote_defaults(&mut open_repository, &repo_details); - // load config from repo - let (load_config_from_repo_open_repository, read_files) = - given::open_repository_for_loading_config_from_repo(&repo_config); - expect::duplicate(&mut open_repository, load_config_from_repo_open_repository); - - // handles_validate_repo_message(&mut open_repository, repo_config.branches()); - let mut repository_factory = MockRepositoryFactory::new(); expect::open_repository(&mut repository_factory, open_repository); fs.dir_create(&repo_details.gitdir)?; - let branch = repo_details.branch.clone(); //when - let (addr, _log) = when::start_actor(repository_factory, repo_details, given::a_forge()); + let (addr, log) = when::start_actor(repository_factory, repo_details, given::a_forge()); addr.send(CloneRepo::new()).await?; System::current().stop(); //then - tracing::debug!("{read_files:#?}"); - let file_name = PathBuf::from(".git-next.toml".to_string()); - read_files - .lock() - .map_err(|e| e.to_string()) - .map(|files| assert_eq!(files.clone(), vec![(branch, file_name)]))?; + log.require_message_containing("send: LoadConfigFromRepo")?; Ok(()) } @@ -142,10 +129,7 @@ async fn opened_repo_with_no_default_push_should_not_proceed() -> TestResult { System::current().stop(); //then - log.lock() - .map_err(|e| e.to_string()) - // doesn't log that it sent any messages to load config or validate the repo - .map(|l| assert_eq!(l.clone(), vec!["open failed"]))?; + log.require_message_containing("open failed")?; Ok(()) } @@ -174,10 +158,7 @@ async fn opened_repo_with_no_default_fetch_should_not_proceed() -> TestResult { System::current().stop(); //then - log.lock() - .map_err(|e| e.to_string()) - // doesn't log that it sent any messages to load config or validate the repo - .map(|l| assert_eq!(l.clone(), vec!["open failed"]))?; + log.require_message_containing("open failed")?; Ok(()) } diff --git a/crates/repo-actor/src/tests/mod.rs b/crates/repo-actor/src/tests/mod.rs index 81d3b21..d80c8a8 100644 --- a/crates/repo-actor/src/tests/mod.rs +++ b/crates/repo-actor/src/tests/mod.rs @@ -22,7 +22,6 @@ use git_next_git as git; use mockall::predicate::eq; use std::{ collections::{BTreeMap, HashMap}, - path::PathBuf, sync::{Arc, Mutex}, };