From 926851db1924e881a6d91e30c3d47c1229c06666 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sun, 9 Jun 2024 09:46:40 +0100 Subject: [PATCH] refactor: rewrite git crate's mock repository --- crates/git/src/repository/mock.rs | 199 +++++++++++------------------ crates/git/src/repository/mod.rs | 9 +- crates/git/src/repository/tests.rs | 145 ++++++++------------- crates/git/src/tests.rs | 32 ++--- 4 files changed, 143 insertions(+), 242 deletions(-) diff --git a/crates/git/src/repository/mock.rs b/crates/git/src/repository/mock.rs index eb252a7..a68bf1a 100644 --- a/crates/git/src/repository/mock.rs +++ b/crates/git/src/repository/mock.rs @@ -6,42 +6,43 @@ use std::{ sync::{Arc, Mutex}, }; -use super::Error; -use crate as git; +use crate::{self as git, Repository}; use crate::{ repository::{ open::{OpenRepository, OpenRepositoryLike}, - Direction, RepositoryLike, Result, + Direction, RepositoryLike, }, GitRemote, RepoDetails, }; use git_next_config as config; #[derive(Debug, Default, Clone)] -pub struct MockRepository(Arc>); +pub struct MockRepository { + open_repos: Arc>>, +} impl MockRepository { pub fn new() -> Self { - Self(Arc::new(Mutex::new(Reality::default()))) + Self { + open_repos: Default::default(), + } } - pub fn can_open_repo(&mut self, gitdir: &config::GitDir) -> Result { - self.0 + + pub fn given_can_be_opened(&mut self, gitdir: &config::GitDir) -> MockOpenRepository { + let open_repo = MockOpenRepository::new(); + #[allow(clippy::unwrap_used)] + self.open_repos .lock() - .map_err(|_| Error::MockLock) - .map(|mut r| r.can_open_repo(gitdir)) + .map(|mut or| or.insert(gitdir.clone(), open_repo.clone())) + .unwrap(); + open_repo } - fn open_repository( - &self, - gitdir: &config::GitDir, - ) -> std::result::Result { - self.0.lock().map_err(|_| Error::MockLock).and_then(|r| { - r.open_repository(gitdir) - .ok_or_else(|| Error::Open(format!("mock - could not open: {}", gitdir))) - }) + + pub fn seal(self) -> (Repository, Self) { + (Repository::Mock(self.clone()), self) } - fn clone_repository( - &self, - ) -> std::result::Result { - todo!() + pub fn unseal(self, _repository: Repository) -> Self { + // drop repository to allow same mutable access to mock repository + self } } impl RepositoryLike for MockRepository { @@ -49,131 +50,83 @@ impl RepositoryLike for MockRepository { &self, gitdir: &config::GitDir, ) -> std::result::Result { - Ok(OpenRepository::Mock(self.open_repository(gitdir)?)) + #[allow(clippy::unwrap_used)] + self.open_repos + .lock() + .map_err(|_| crate::repository::Error::MockLock) + .map(|or| or.get(gitdir).cloned()) + .transpose() + .unwrap_or_else(|| Err(crate::repository::Error::InvalidGitDir(gitdir.clone()))) + .map(|mor| mor.into()) } fn git_clone( &self, _repo_details: &RepoDetails, ) -> std::result::Result { - Ok(OpenRepository::Mock(self.clone_repository()?)) + todo!("MockRepository::git_clone") } } -#[derive(Debug, Default)] -pub struct Reality { - openable_repos: HashMap, -} -impl Reality { - pub fn can_open_repo(&mut self, gitdir: &config::GitDir) -> MockOpenRepository { - let mor = self.openable_repos.get(gitdir); - match mor { - Some(mor) => mor.clone(), - None => { - let mor = MockOpenRepository::default(); - self.openable_repos.insert(gitdir.clone(), mor.clone()); - mor - } - } - } - pub fn open_repository(&self, gitdir: &config::GitDir) -> Option { - self.openable_repos.get(gitdir).cloned() - } -} -#[derive(Clone, Debug, Default)] -pub struct InnerMockOpenRepository { - default_push_remote: Option, - default_fetch_remote: Option, -} - #[derive(Clone, Debug, Default)] pub struct MockOpenRepository { - inner: Arc>, + default_push_remote: Arc>>, + default_fetch_remote: Arc>>, } -impl std::ops::Deref for MockOpenRepository { - type Target = Arc>; - - fn deref(&self) -> &Self::Target { - &self.inner +impl MockOpenRepository { + pub fn new() -> Self { + Self::default() + } + pub fn given_has_default_remote(&mut self, direction: Direction, remote: Option) { + #[allow(clippy::unwrap_used)] + match direction { + Direction::Push => self + .default_push_remote + .lock() + .map(|mut o| match remote { + Some(gr) => o.replace(gr), + None => o.take(), + }) + .unwrap(), + Direction::Fetch => self + .default_fetch_remote + .lock() + .map(|mut o| match remote { + Some(gr) => o.replace(gr), + None => o.take(), + }) + .unwrap(), + }; } } - -impl InnerMockOpenRepository { - pub fn has_default_remote(&mut self, direction: Direction, git_remote: GitRemote) { - match direction { - Direction::Push => self.default_push_remote.replace(git_remote), - Direction::Fetch => self.default_fetch_remote.replace(git_remote), - }; +impl From for OpenRepository { + fn from(value: MockOpenRepository) -> Self { + Self::Mock(value) } } #[allow(clippy::unwrap_used)] impl OpenRepositoryLike for MockOpenRepository { fn remote_branches(&self) -> git::push::Result> { - self.inner - .lock() - .map(|inner| inner.remote_branches()) - .unwrap() - } - fn find_default_remote(&self, direction: Direction) -> Option { - self.inner - .lock() - .map(|inner| inner.find_default_remote(direction)) - .unwrap() - } - - fn fetch(&self) -> core::result::Result<(), crate::fetch::Error> { - self.inner.lock().map(|inner| inner.fetch()).unwrap() - } - - fn push( - &self, - repo_details: &RepoDetails, - branch_name: &git_next_config::BranchName, - to_commit: &crate::GitRef, - force: &crate::push::Force, - ) -> core::result::Result<(), crate::push::Error> { - self.inner - .lock() - .map(|inner| inner.push(repo_details, branch_name, to_commit, force)) - .unwrap() - } - - fn commit_log( - &self, - branch_name: &git_next_config::BranchName, - find_commits: &[git::Commit], - ) -> core::result::Result, git::commit::log::Error> { - self.inner - .lock() - .map(|inner| inner.commit_log(branch_name, find_commits)) - .unwrap() - } - - fn read_file( - &self, - branch_name: &git_next_config::BranchName, - file_name: &str, - ) -> git::file::Result { - self.inner - .lock() - .map(|inner| inner.read_file(branch_name, file_name)) - .unwrap() - } -} -impl OpenRepositoryLike for InnerMockOpenRepository { - fn remote_branches(&self) -> git::push::Result> { - todo!(); + todo!("MockOpenRepository::remote_branched") } fn find_default_remote(&self, direction: Direction) -> Option { match direction { - Direction::Push => self.default_push_remote.clone(), - Direction::Fetch => self.default_fetch_remote.clone(), + Direction::Push => self + .default_push_remote + .lock() + .map(|r| r.clone()) + .unwrap_or(None), + Direction::Fetch => self + .default_fetch_remote + .lock() + .map(|r| r.clone()) + .unwrap_or(None), } } fn fetch(&self) -> core::result::Result<(), crate::fetch::Error> { - todo!() + todo!("MockOpenRepository::fetch") } fn push( @@ -183,15 +136,15 @@ impl OpenRepositoryLike for InnerMockOpenRepository { _to_commit: &crate::GitRef, _force: &crate::push::Force, ) -> core::result::Result<(), crate::push::Error> { - todo!() + todo!("MockOpenRepository::push") } fn commit_log( &self, _branch_name: &git_next_config::BranchName, _find_commits: &[git::Commit], - ) -> core::result::Result, crate::commit::log::Error> { - todo!() + ) -> core::result::Result, git::commit::log::Error> { + todo!("MockOpenRepository::commit_log") } fn read_file( @@ -199,6 +152,6 @@ impl OpenRepositoryLike for InnerMockOpenRepository { _branch_name: &git_next_config::BranchName, _file_name: &str, ) -> git::file::Result { - todo!() + todo!("MockOpenRepository::read_file") } } diff --git a/crates/git/src/repository/mod.rs b/crates/git/src/repository/mod.rs index 71b1ecd..8d40348 100644 --- a/crates/git/src/repository/mod.rs +++ b/crates/git/src/repository/mod.rs @@ -9,10 +9,11 @@ mod tests; use git_next_config as config; use git_next_config::GitDir; +pub use mock::MockRepository; pub use open::OpenRepository; use tracing::info; -use crate::{repository::mock::MockRepository, validation::repo::validate_repo}; +use crate::validation::repo::validate_repo; use super::RepoDetails; @@ -24,9 +25,9 @@ pub enum Repository { pub const fn new() -> Repository { Repository::Real } -pub fn mock() -> (Repository, MockRepository) { - let mock_repository = MockRepository::new(); - (Repository::Mock(mock_repository.clone()), mock_repository) +pub fn mock() -> MockRepository { + MockRepository::new() + // (Repository::Mock(mock_repository.clone()), mock_repository) } /// Opens a repository, cloning if necessary diff --git a/crates/git/src/repository/tests.rs b/crates/git/src/repository/tests.rs index 6de807c..337df18 100644 --- a/crates/git/src/repository/tests.rs +++ b/crates/git/src/repository/tests.rs @@ -1,14 +1,12 @@ -mod validate { +use crate as git; +mod validate { + use crate::{validation::repo::validate_repo, GitRemote, RepoDetails}; + + use super::*; use assert2::let_assert; use git_next_config::{ForgeDetails, GitDir, Hostname, RepoPath}; - use crate::{ - repository, - validation::{self, repo::validate_repo}, - GitRemote, RepoDetails, - }; - #[test] fn should_ok_a_valid_repo() { let repo_details = RepoDetails::default() @@ -22,25 +20,17 @@ mod validate { RepoPath::new("kemitix/test".to_string()), ); - let (repository, mut reality) = repository::mock(); - let_assert!( - Ok(_) = reality.can_open_repo(&gitdir).map( - #[allow(clippy::significant_drop_tightening)] - |open_repo| { - let_assert!( - Ok(_) = open_repo.lock().map(|mut open_repo| { - open_repo - .has_default_remote(repository::Direction::Push, remote.clone()); - open_repo.has_default_remote(repository::Direction::Fetch, remote); - }) - ); - } - ) - ); - + let mut mock_repository = git::repository::mock(); + { + let mut mock_open_repo = mock_repository.given_can_be_opened(&gitdir); + mock_open_repo + .given_has_default_remote(git::repository::Direction::Push, Some(remote.clone())); + mock_open_repo + .given_has_default_remote(git::repository::Direction::Fetch, Some(remote)); + } + let (repository, _mock_repository) = mock_repository.seal(); let_assert!(Ok(open_repository) = repository.open(&gitdir)); - - let_assert!(Ok(_) = validation::repo::validate_repo(&open_repository, &repo_details)); + let_assert!(Ok(_) = validate_repo(&open_repository, &repo_details)); } #[test] @@ -56,23 +46,15 @@ mod validate { RepoPath::new("kemitix/test".to_string()), ); - let (repository, mut reality) = repository::mock(); - let_assert!( - Ok(_) = reality.can_open_repo(&gitdir).map( - #[allow(clippy::significant_drop_tightening)] - |open_repo| { - let_assert!( - Ok(_) = open_repo.lock().map(|mut open_repo| { - // INFO: open_repo.has_default_remote(Direction::Push, remote.clone()); - open_repo.has_default_remote(repository::Direction::Fetch, remote); - }) - ); - } - ) - ); - + let mut mock_repository = git::repository::mock(); + { + let mut mock_open_repo = mock_repository.given_can_be_opened(&gitdir); + mock_open_repo.given_has_default_remote(git::repository::Direction::Push, None); + mock_open_repo + .given_has_default_remote(git::repository::Direction::Fetch, Some(remote)); + } + let (repository, _mock_repository) = mock_repository.seal(); let_assert!(Ok(open_repository) = repository.open(&gitdir)); - let_assert!(Err(_) = validate_repo(&open_repository, &repo_details)); } #[test] @@ -88,23 +70,15 @@ mod validate { RepoPath::new("kemitix/test".to_string()), ); - let (repository, mut reality) = repository::mock(); - let_assert!( - Ok(_) = reality.can_open_repo(&gitdir).map( - #[allow(clippy::significant_drop_tightening)] - |open_repo| { - let_assert!( - Ok(_) = open_repo.lock().map(|mut open_repo| { - open_repo.has_default_remote(repository::Direction::Push, remote); - // INFO: open_repo.has_default_remote(Direction::Fetch, remote); - }) - ); - } - ) - ); - + let mut mock_repository = git::repository::mock(); + { + let mut mock_open_repo = mock_repository.given_can_be_opened(&gitdir); + mock_open_repo.given_has_default_remote(git::repository::Direction::Push, None); + mock_open_repo + .given_has_default_remote(git::repository::Direction::Fetch, Some(remote)); + } + let (repository, _mock_repository) = mock_repository.seal(); let_assert!(Ok(open_repository) = repository.open(&gitdir)); - let_assert!(Err(_) = validate_repo(&open_repository, &repo_details)); } #[test] @@ -124,23 +98,16 @@ mod validate { RepoPath::new("kemitix/other".to_string()), ); - let (repository, mut reality) = repository::mock(); - let_assert!( - Ok(_) = reality.can_open_repo(&gitdir).map( - #[allow(clippy::significant_drop_tightening)] - |open_repo| { - let_assert!( - Ok(_) = open_repo.lock().map(|mut open_repo| { - open_repo.has_default_remote(repository::Direction::Push, other_remote); - open_repo.has_default_remote(repository::Direction::Fetch, remote); - }) - ); - } - ) - ); - + let mut mock_repository = git::repository::mock(); + { + let mut mock_open_repo = mock_repository.given_can_be_opened(&gitdir); + mock_open_repo + .given_has_default_remote(git::repository::Direction::Push, Some(other_remote)); + mock_open_repo + .given_has_default_remote(git::repository::Direction::Fetch, Some(remote)); + } + let (repository, _mock_repository) = mock_repository.seal(); let_assert!(Ok(open_repository) = repository.open(&gitdir)); - let_assert!(Err(_) = validate_repo(&open_repository, &repo_details)); } #[test] @@ -160,33 +127,25 @@ mod validate { RepoPath::new("kemitix/other".to_string()), ); - let (repository, mut reality) = repository::mock(); - let_assert!( - Ok(_) = reality.can_open_repo(&gitdir).map( - #[allow(clippy::significant_drop_tightening)] - |open_repo| { - let_assert!( - Ok(_) = open_repo.lock().map(|mut open_repo| { - open_repo.has_default_remote(repository::Direction::Push, remote); - open_repo - .has_default_remote(repository::Direction::Fetch, other_remote); - }) - ); - } - ) - ); - + let mut mock_repository = git::repository::mock(); + { + let mut mock_open_repo = mock_repository.given_can_be_opened(&gitdir); + mock_open_repo.given_has_default_remote(git::repository::Direction::Push, Some(remote)); + mock_open_repo + .given_has_default_remote(git::repository::Direction::Fetch, Some(other_remote)); + } + let (repository, _mock_repository) = mock_repository.seal(); let_assert!(Ok(open_repository) = repository.open(&gitdir)); - let_assert!(Err(_) = validate_repo(&open_repository, &repo_details)); } } mod git_clone { + use super::*; use assert2::let_assert; use git_next_config::{ForgeDetails, GitDir, Hostname, RepoPath}; - use crate::{repository, GitRemote, RepoDetails}; + use crate::{GitRemote, RepoDetails}; #[test] #[ignore] // slow test ~1.5 seconds @@ -200,7 +159,9 @@ mod git_clone { .with_gitdir(GitDir::new(fs.base())) .with_repo_path(RepoPath::new("kemitix/git-next".to_string())); let_assert!(Ok(open_repo) = r.git_clone(&repo_details)); - let_assert!(Some(remote) = open_repo.find_default_remote(repository::Direction::Fetch)); + let_assert!( + Some(remote) = open_repo.find_default_remote(git::repository::Direction::Fetch) + ); assert_eq!( remote, GitRemote::new( diff --git a/crates/git/src/tests.rs b/crates/git/src/tests.rs index 89321ec..6aabdd1 100644 --- a/crates/git/src/tests.rs +++ b/crates/git/src/tests.rs @@ -81,9 +81,7 @@ mod gitremote { } } mod push { - use crate::{commit, Commit, GitRef}; - - use crate::push::Force; + use crate::{commit, push::Force, Commit, GitRef}; #[test] fn force_no_should_display() { @@ -174,26 +172,6 @@ mod repo_details { ); } } -// mod branch { -// use super::*; -// use assert2::let_assert; -// #[test] -// fn reset_should_fetch_then_push() { -// // let repository = given::a_mock_open_repository(); -// let fs = given::a_filesystem(); -// let repo_detauls = given::repo_details(&fs); -// let repository = given::a_mock_open_repository(); -// let_assert!( -// Ok(result) = git::branch::reset( -// repository, -// &repo_details, -// &repo_details.branch, -// git_ref, -// force -// ) -// ); -// } -// } mod given { #![allow(dead_code)] // @@ -356,4 +334,12 @@ mod given { gitdir, ) } + + // 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) + // } }