From 07f2357dcd87338c4fc3f34d9f356157aa9ae57f Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sun, 9 Jun 2024 10:21:09 +0100 Subject: [PATCH] WIP: tests: add more tests to git crate --- crates/config/src/branch_name.rs | 4 +- crates/git/Cargo.toml | 1 + crates/git/src/file.rs | 2 +- crates/git/src/repo_details.rs | 4 +- crates/git/src/repository/mock.rs | 40 +++++++-- crates/git/src/repository/mod.rs | 2 + crates/git/src/repository/open/oreal.rs | 5 +- crates/git/src/repository/real.rs | 1 + crates/git/src/repository/tests.rs | 106 ++++++++++++++++++++++++ crates/git/src/tests.rs | 74 +++++++++++++---- crates/repo-actor/src/lib.rs | 2 +- 11 files changed, 210 insertions(+), 31 deletions(-) diff --git a/crates/config/src/branch_name.rs b/crates/config/src/branch_name.rs index 5086151..92a5bd5 100644 --- a/crates/config/src/branch_name.rs +++ b/crates/config/src/branch_name.rs @@ -1,5 +1,7 @@ +use derive_more::Display; + /// The name of a Branch -#[derive(Clone, Default, Debug, Hash, PartialEq, Eq, derive_more::Display)] +#[derive(Clone, Default, Debug, Hash, PartialEq, Eq, Display, PartialOrd, Ord)] pub struct BranchName(String); impl BranchName { pub fn new(str: impl Into) -> Self { diff --git a/crates/git/Cargo.toml b/crates/git/Cargo.toml index 21df2be..48a794c 100644 --- a/crates/git/Cargo.toml +++ b/crates/git/Cargo.toml @@ -55,6 +55,7 @@ actix = { workspace = true } # Testing assert2 = { workspace = true } rand = { workspace = true } +pretty_assertions = { workspace = true } [lints.clippy] nursery = { level = "warn", priority = -1 } diff --git a/crates/git/src/file.rs b/crates/git/src/file.rs index 34be9d0..ea93e3b 100644 --- a/crates/git/src/file.rs +++ b/crates/git/src/file.rs @@ -37,7 +37,7 @@ pub enum Error { NoTreeInCommit(String), #[error("no .git-next.toml file found in repo")] - NoGitNextToml, + FileNotFound, #[error("find reference: {0}")] FindReference(String), diff --git a/crates/git/src/repo_details.rs b/crates/git/src/repo_details.rs index d993bf7..f0d761d 100644 --- a/crates/git/src/repo_details.rs +++ b/crates/git/src/repo_details.rs @@ -7,9 +7,7 @@ use super::{Generation, GitRemote}; /// The derived information about a repo, used to interact with it #[derive(Clone, Default, Debug, derive_more::Display, derive_with::With)] -#[display("gen-{}:{}:{}/{}:{}@{}/{}@{}", generation, forge.forge_type(), - forge.forge_alias(), repo_alias, forge.user(), forge.hostname(), repo_path, - branch)] +#[display("gen-{}:{}:{}/{}", generation, forge.forge_type(), forge.forge_alias(), repo_alias )] pub struct RepoDetails { pub generation: Generation, pub repo_alias: RepoAlias, diff --git a/crates/git/src/repository/mock.rs b/crates/git/src/repository/mock.rs index a68bf1a..3e3b5b6 100644 --- a/crates/git/src/repository/mock.rs +++ b/crates/git/src/repository/mock.rs @@ -44,6 +44,12 @@ impl MockRepository { // drop repository to allow same mutable access to mock repository self } + pub fn get(&self, gitdir: &config::GitDir) -> Option { + self.open_repos + .lock() + .map(|or| or.get(gitdir).cloned()) + .unwrap_or(None) + } } impl RepositoryLike for MockRepository { fn open( @@ -72,6 +78,7 @@ impl RepositoryLike for MockRepository { pub struct MockOpenRepository { default_push_remote: Arc>>, default_fetch_remote: Arc>>, + operations: Arc>>, } impl MockOpenRepository { pub fn new() -> Self { @@ -98,6 +105,13 @@ impl MockOpenRepository { .unwrap(), }; } + + pub fn operations(&self) -> Vec { + self.operations + .lock() + .map(|operations| operations.clone()) + .unwrap_or_default() + } } impl From for OpenRepository { fn from(value: MockOpenRepository) -> Self { @@ -126,17 +140,31 @@ impl OpenRepositoryLike for MockOpenRepository { } fn fetch(&self) -> core::result::Result<(), crate::fetch::Error> { - todo!("MockOpenRepository::fetch") + self.operations + .lock() + .map_err(|_| crate::fetch::Error::Lock) + .map(|mut operations| operations.push("fetch".to_string()))?; + Ok(()) } fn push( &self, - _repo_details: &RepoDetails, - _branch_name: &git_next_config::BranchName, - _to_commit: &crate::GitRef, - _force: &crate::push::Force, + repo_details: &RepoDetails, + branch_name: &git_next_config::BranchName, + to_commit: &crate::GitRef, + force: &crate::push::Force, ) -> core::result::Result<(), crate::push::Error> { - todo!("MockOpenRepository::push") + let forge_alias = repo_details.forge.forge_alias(); + let repo_alias = &repo_details.repo_alias; + self.operations + .lock() + .map_err(|_| crate::fetch::Error::Lock) + .map(|mut operations| { + operations.push(format!( + "push fa:{forge_alias} ra:{repo_alias} bn:{branch_name} tc:{to_commit} f:{force}" + )) + })?; + Ok(()) } fn commit_log( diff --git a/crates/git/src/repository/mod.rs b/crates/git/src/repository/mod.rs index 8d40348..d71ccd3 100644 --- a/crates/git/src/repository/mod.rs +++ b/crates/git/src/repository/mod.rs @@ -9,6 +9,7 @@ mod tests; use git_next_config as config; use git_next_config::GitDir; +pub use mock::MockOpenRepository; pub use mock::MockRepository; pub use open::OpenRepository; use tracing::info; @@ -32,6 +33,7 @@ pub fn mock() -> MockRepository { /// Opens a repository, cloning if necessary #[tracing::instrument(skip_all)] +#[cfg(not(tarpaulin_include))] // requires network access to either clone new and/or fetch. pub fn open( repository: &Repository, repo_details: &RepoDetails, diff --git a/crates/git/src/repository/open/oreal.rs b/crates/git/src/repository/open/oreal.rs index be921f4..92069fe 100644 --- a/crates/git/src/repository/open/oreal.rs +++ b/crates/git/src/repository/open/oreal.rs @@ -45,6 +45,7 @@ impl super::OpenRepositoryLike for RealOpenRepository { } #[tracing::instrument(skip_all)] + #[cfg(not(tarpaulin_include))] // would require writing to external service fn fetch(&self) -> Result<(), git::fetch::Error> { let Ok(repository) = self.0.lock() else { #[cfg(not(tarpaulin_include))] // don't test mutex lock failure @@ -171,8 +172,8 @@ impl super::OpenRepositoryLike for RealOpenRepository { let commit = obj.into_commit(); let tree = commit.tree()?; let ent = tree - .find_entry(".git-next.toml") - .ok_or(git::file::Error::NoGitNextToml)?; + .find_entry(file_name) + .ok_or(git::file::Error::FileNotFound)?; let fobj = ent.object()?; let blob = fobj.into_blob().take_data(); let content = String::from_utf8(blob)?; diff --git a/crates/git/src/repository/real.rs b/crates/git/src/repository/real.rs index 3284c12..7b54f1d 100644 --- a/crates/git/src/repository/real.rs +++ b/crates/git/src/repository/real.rs @@ -16,6 +16,7 @@ impl RepositoryLike for RealRepository { } #[tracing::instrument(skip_all)] + #[cfg(not(tarpaulin_include))] // requires external server fn git_clone(&self, repo_details: &RepoDetails) -> Result { tracing::info!("creating"); use secrecy::ExposeSecret; diff --git a/crates/git/src/repository/tests.rs b/crates/git/src/repository/tests.rs index 337df18..39ca73a 100644 --- a/crates/git/src/repository/tests.rs +++ b/crates/git/src/repository/tests.rs @@ -1,4 +1,5 @@ use crate as git; +use git_next_config as config; mod validate { use crate::{validation::repo::validate_repo, GitRemote, RepoDetails}; @@ -171,3 +172,108 @@ mod git_clone { ); } } +mod open { + + use assert2::let_assert; + use git_next_config::{BranchName, GitDir}; + + use crate::{ + repository::{real::RealRepository, RepositoryLike as _}, + tests::given, + }; + + use super::*; + + mod remote_branches { + use super::*; + #[test] + // assumes running in the git-next repo which should have main, next and dev as remote branches + fn should_return_remote_branches() { + let open_repo = given_open_real_repository(); + let_assert!(Ok(mut remote_branches) = open_repo.remote_branches()); + remote_branches.sort(); + let mut expected = [ + BranchName::new("main"), + BranchName::new("next"), + BranchName::new("dev"), + ]; + expected.sort(); + assert_eq!(remote_branches, expected); + } + } + mod commit_log { + use super::*; + + #[test] + // assumes running in the git-next repo which should have main, next and dev as remote branches + fn should_return_single_item_in_commit_log_when_not_searching() { + let open_repo = given_open_real_repository(); + let_assert!(Ok(result) = open_repo.commit_log(&config::BranchName::new("dev"), &[])); + assert_eq!(result.len(), 1) + } + + #[test] + // assumes running in the git-next repo which should have main, next and dev as remote branches + fn should_return_capacity_50_in_commit_log_when_searching_for_garbage() { + let open_repo = given_open_real_repository(); + let_assert!( + Ok(result) = + open_repo.commit_log(&config::BranchName::new("dev"), &[given::a_commit()]) + ); + assert_eq!(result.len(), 50) + } + + #[test] + // assumes running in the git-next repo which should have main, next and dev as remote branches + fn should_return_25_in_commit_log_when_searching_for_25th_item() { + let open_repo = given_open_real_repository(); + // search to garbage to get full page of 50 items + let_assert!( + Ok(long_list) = + open_repo.commit_log(&config::BranchName::new("dev"), &[given::a_commit()]) + ); + // pick the 25th item + let search = &long_list[24]; // zero-based + // search for the 25th item + let_assert!( + Ok(result) = + open_repo.commit_log(&config::BranchName::new("dev"), &[search.clone()]) + ); + // returns + assert_eq!(result.len(), 25) + } + } + mod read_file { + use super::*; + + #[test] + // assumes running in the git-next repo which should have main, next and dev as remote branches + fn should_return_file() { + let open_repo = given_open_real_repository(); + let_assert!( + Ok(result) = + open_repo.read_file(&config::BranchName::new("dev"), "server-default.toml") + ); + let expected = include_str!("../../../../server-default.toml"); + assert_eq!(result, expected); + } + + #[test] + // assumes running in the git-next repo which should have main, next and dev as remote branches + fn should_error_on_missing_file() { + let open_repo = given_open_real_repository(); + let_assert!( + Err(err) = open_repo.read_file(&config::BranchName::new("dev"), &given::a_name()) + ); + assert!(matches!(err, git::file::Error::FileNotFound)); + } + } + + fn given_open_real_repository() -> git::OpenRepository { + // assumes the current directory is a clone of this repo + let_assert!(Ok(current_dir) = std::env::current_dir()); // ./crates/git + let gitdir: GitDir = current_dir.join("../..").into(); // cd back to project root + let_assert!(Ok(real_open_repository) = RealRepository.open(&gitdir)); + real_open_repository + } +} diff --git a/crates/git/src/tests.rs b/crates/git/src/tests.rs index 6aabdd1..079f7f7 100644 --- a/crates/git/src/tests.rs +++ b/crates/git/src/tests.rs @@ -81,23 +81,59 @@ mod gitremote { } } mod push { - use crate::{commit, push::Force, Commit, GitRef}; + use super::*; + use crate::GitRef; #[test] fn force_no_should_display() { - assert_eq!(Force::No.to_string(), "fast-forward") + assert_eq!(git::push::Force::No.to_string(), "fast-forward") } + #[test] fn force_from_should_display() { - let commit = Commit::new( - commit::Sha::new("sha".to_string()), - commit::Message::new("message".to_string()), - ); + let sha = given::a_name(); + let commit = given::a_commit_with_sha(&git::commit::Sha::new(sha.clone())); assert_eq!( - Force::From(GitRef::from(commit)).to_string(), - "force-if-from:sha" + git::push::Force::From(GitRef::from(commit)).to_string(), + format!("force-if-from:{sha}") ) } + + mod reset { + use super::*; + use crate::{tests::given, OpenRepository}; + use assert2::let_assert; + + #[test] + fn should_perform_a_fetch_then_push() { + let fs = given::a_filesystem(); + let (mock_open_repository, gitdir, mock_repository) = given::an_open_repository(&fs); + let open_repository: OpenRepository = mock_open_repository.into(); + let repo_details = given::repo_details(&fs); + let branch_name = &repo_details.branch; + let commit = given::a_commit(); + let gitref = GitRef::from(commit); + let_assert!( + Ok(_) = git::push::reset( + &open_repository, + &repo_details, + branch_name, + &gitref, + &git::push::Force::No + ) + ); + let_assert!(Some(mock_open_repository) = mock_repository.get(&gitdir)); + let operations = mock_open_repository.operations(); + let forge_alias = repo_details.forge.forge_alias(); + let repo_alias = &repo_details.repo_alias; + let to_commit = gitref; + let force = "fast-forward"; + assert_eq!( + operations, + vec![format!("fetch"), format!("push fa:{forge_alias} ra:{repo_alias} bn:{branch_name} tc:{to_commit} f:{force}")] + ); + } + } } mod repo_details { @@ -172,10 +208,13 @@ mod repo_details { ); } } -mod given { +pub mod given { #![allow(dead_code)] // - use crate as git; + use crate::{ + self as git, + repository::{MockOpenRepository, MockRepository}, + }; use config::{ BranchName, ForgeAlias, ForgeConfig, ForgeType, GitDir, RepoAlias, RepoBranches, ServerRepoConfig, WebhookAuth, WebhookId, @@ -335,11 +374,12 @@ mod given { ) } - // pub fn an_open_repository(fs: &kxio::fs::FileSystem) -> (OpenRepository, MockRepository) { - // let (repo, mock) = git::repository::mock(); - // - // let gitdir = a_git_dir(fs); - // let op = repo.open(&gitdir).unwrap(); - // (op, mock) - // } + pub fn an_open_repository( + fs: &kxio::fs::FileSystem, + ) -> (MockOpenRepository, GitDir, MockRepository) { + let mut mock = git::repository::mock(); + let gitdir = a_git_dir(fs); + let or = mock.given_can_be_opened(&gitdir); + (or, gitdir, mock) + } } diff --git a/crates/repo-actor/src/lib.rs b/crates/repo-actor/src/lib.rs index fbcd170..3bf1815 100644 --- a/crates/repo-actor/src/lib.rs +++ b/crates/repo-actor/src/lib.rs @@ -93,7 +93,7 @@ impl Actor for RepoActor { pub struct CloneRepo; impl Handler for RepoActor { type Result = (); - #[tracing::instrument(name = "RepoActor::CloneRepo", skip_all, fields(repo = %self.repo_details, gitdir = %self.repo_details.gitdir))] + #[tracing::instrument(name = "RepoActor::CloneRepo", skip_all, fields(repo = %self.repo_details /*, gitdir = %self.repo_details.gitdir */))] fn handle(&mut self, _msg: CloneRepo, ctx: &mut Self::Context) -> Self::Result { let gitdir = self.repo_details.gitdir.clone(); match git::repository::open(&self.repository, &self.repo_details, gitdir) {