remove '_pure' functions in favor of creating a full handler in tests.

The `pure` functions were from a time where a `Handler` couldn't be instantiated
in full for tests, but that is not the case anymore, so there isn't any use
for the added complexity.
This commit is contained in:
Sebastian Thiel 2024-04-15 16:37:36 +02:00
parent 62b1c49372
commit fb9db89e1b
No known key found for this signature in database
GPG Key ID: 9CB5EE7895E8268B
3 changed files with 79 additions and 120 deletions

View File

@ -199,7 +199,7 @@ impl Handler {
let this = self.clone();
move || this.push_gb_data(project_id)
});
self.push_project_to_gitbutler(project_id).await?;
self.push_project_to_gitbutler(project_id, 1000).await?;
push_gb_data.await??;
Ok(())
}

View File

@ -1,4 +1,3 @@
use std::path::Path;
use std::time;
use anyhow::{Context, Result};
@ -13,34 +12,21 @@ use gitbutler_core::{
use itertools::Itertools;
impl super::Handler {
pub(super) async fn push_project_to_gitbutler(&self, project_id: ProjectId) -> Result<()> {
Self::push_project_to_gitbutler_pure(
&self.local_data_dir,
&self.projects,
&self.users,
project_id,
1000,
)
.await
}
}
/// Currently required to make functionality testable without requiring a `Handler` with all of its state.
impl super::Handler {
pub async fn push_project_to_gitbutler_pure(
local_data_dir: &Path,
projects: &projects::Controller,
users: &users::Controller,
pub async fn push_project_to_gitbutler(
&self,
project_id: ProjectId,
batch_size: usize,
) -> Result<()> {
let project = projects.get(&project_id).context("failed to get project")?;
let project = self
.projects
.get(&project_id)
.context("failed to get project")?;
if !project.is_sync_enabled() || !project.has_code_url() {
return Ok(());
}
let user = users.get_user()?;
let user = self.users.get_user()?;
let project_repository =
project_repository::Repository::open(&project).context("failed to open repository")?;
let gb_code_last_commit = project
@ -48,8 +34,11 @@ impl super::Handler {
.as_ref()
.map(|state| &state.id)
.copied();
let gb_repository =
gb_repository::Repository::open(local_data_dir, &project_repository, user.as_ref())?;
let gb_repository = gb_repository::Repository::open(
&self.local_data_dir,
&project_repository,
user.as_ref(),
)?;
let default_target = gb_repository
.default_target()
.context("failed to open gb repo")?
@ -57,16 +46,16 @@ impl super::Handler {
let target_changed = gb_code_last_commit.map_or(true, |id| id != default_target.sha);
if target_changed {
match Self::push_target(
projects,
&project_repository,
&default_target,
gb_code_last_commit,
project_id,
user.as_ref(),
batch_size,
)
.await
match self
.push_target(
&project_repository,
&default_target,
gb_code_last_commit,
project_id,
user.as_ref(),
batch_size,
)
.await
{
Ok(()) => {}
Err(project_repository::RemoteError::Network) => return Ok(()),
@ -84,16 +73,19 @@ impl super::Handler {
.await??;
// make sure last push time is updated
Self::update_project(projects, project_id, default_target.sha).await?;
self.update_project(project_id, default_target.sha).await?;
Ok(())
}
}
/// Currently required to make functionality testable without requiring a `Handler` with all of its state.
impl super::Handler {
async fn update_project(
projects: &projects::Controller,
&self,
project_id: Id<projects::Project>,
id: Oid,
) -> Result<(), project_repository::RemoteError> {
projects
self.projects
.update(&projects::UpdateRequest {
id: project_id,
gitbutler_code_push_state: Some(CodePushState {
@ -108,7 +100,7 @@ impl super::Handler {
}
async fn push_target(
projects: &projects::Controller,
&self,
project_repository: &project_repository::Repository,
default_target: &gitbutler_core::virtual_branches::target::Target,
gb_code_last_commit: Option<Oid>,
@ -134,7 +126,7 @@ impl super::Handler {
let refspec = format!("+{}:refs/push-tmp/{}", id, project_id);
project_repository.push_to_gitbutler_server(user, &[&refspec])?;
Self::update_project(projects, project_id, *id).await?;
self.update_project(project_id, *id).await?;
tracing::info!(
%project_id,

View File

@ -2,10 +2,10 @@ use std::{collections::HashMap, path::PathBuf};
use anyhow::Result;
use gitbutler_core::{git, project_repository::LogUntil, projects};
use gitbutler_tauri::watcher::Handler;
use crate::watcher::handler::support::Fixture;
use crate::watcher::handler::test_remote_repository;
use gitbutler_testsupport::{virtual_branches::set_test_target, Case, Suite};
use gitbutler_testsupport::{virtual_branches::set_test_target, Case};
fn log_walk(repo: &git2::Repository, head: git::Oid) -> Vec<git::Oid> {
let mut walker = repo.revwalk().unwrap();
@ -15,8 +15,9 @@ fn log_walk(repo: &git2::Repository, head: git::Oid) -> Vec<git::Oid> {
#[tokio::test]
async fn push_error() -> Result<()> {
let suite = Suite::default();
let Case { project, .. } = &suite.new_case();
let mut fixture = Fixture::default();
let handler = fixture.new_handler();
let Case { project, .. } = &fixture.new_case();
let api_project = projects::ApiProject {
name: "test-sync".to_string(),
@ -29,7 +30,7 @@ async fn push_error() -> Result<()> {
sync: true,
};
suite
fixture
.projects
.update(&projects::UpdateRequest {
id: project.id,
@ -38,40 +39,30 @@ async fn push_error() -> Result<()> {
})
.await?;
let res = Handler::push_project_to_gitbutler_pure(
suite.local_app_data(),
&suite.projects,
&suite.users,
project.id,
100,
)
.await;
res.unwrap_err();
let res = handler.push_project_to_gitbutler(project.id, 100).await;
let err = res.unwrap_err();
assert_eq!(err.to_string(), "failed to get default target");
Ok(())
}
#[tokio::test]
async fn push_simple() -> Result<()> {
let suite = Suite::default();
let mut fixture = Fixture::default();
let handler = fixture.new_handler();
let Case {
project,
gb_repository,
project_repository,
..
} = &suite.new_case_with_files(HashMap::from([(PathBuf::from("test.txt"), "test")]));
suite.sign_in();
} = &fixture.new_case_with_files(HashMap::from([(PathBuf::from("test.txt"), "test")]));
fixture.sign_in();
set_test_target(gb_repository, project_repository).unwrap();
let target_id = gb_repository.default_target().unwrap().unwrap().sha;
let reference = project_repository.l(target_id, LogUntil::End).unwrap();
let (cloud_code, _tmp) = test_remote_repository()?;
let api_project = projects::ApiProject {
name: "test-sync".to_string(),
description: None,
@ -83,7 +74,7 @@ async fn push_simple() -> Result<()> {
sync: true,
};
suite
fixture
.projects
.update(&projects::UpdateRequest {
id: project.id,
@ -95,15 +86,10 @@ async fn push_simple() -> Result<()> {
cloud_code.find_commit(target_id.into()).unwrap_err();
{
Handler::push_project_to_gitbutler_pure(
suite.local_app_data(),
&suite.projects,
&suite.users,
project.id,
10,
)
.await
.unwrap();
handler
.push_project_to_gitbutler(project.id, 10)
.await
.unwrap();
}
cloud_code.find_commit(target_id.into()).unwrap();
@ -113,7 +99,7 @@ async fn push_simple() -> Result<()> {
assert_eq!(reference, pushed);
assert_eq!(
suite
fixture
.projects
.get(&project.id)
.unwrap()
@ -128,16 +114,16 @@ async fn push_simple() -> Result<()> {
#[tokio::test]
async fn push_remote_ref() -> Result<()> {
let suite = Suite::default();
let mut fixture = Fixture::default();
let handler = fixture.new_handler();
let Case {
project,
gb_repository,
project_repository,
..
} = &suite.new_case();
suite.sign_in();
} = &fixture.new_case();
fixture.sign_in();
set_test_target(gb_repository, project_repository).unwrap();
let (cloud_code, _tmp) = test_remote_repository()?;
@ -182,7 +168,7 @@ async fn push_remote_ref() -> Result<()> {
sync: true,
};
suite
fixture
.projects
.update(&projects::UpdateRequest {
id: project.id,
@ -192,19 +178,13 @@ async fn push_remote_ref() -> Result<()> {
.await?;
{
Handler::push_project_to_gitbutler_pure(
suite.local_app_data(),
&suite.projects,
&suite.users,
project.id,
10,
)
.await
.unwrap();
handler
.push_project_to_gitbutler(project.id, 10)
.await
.unwrap();
}
cloud_code.find_commit(last_commit).unwrap();
Ok(())
}
@ -255,15 +235,16 @@ fn create_test_commits(repo: &git::Repository, commits: usize) -> git::Oid {
#[tokio::test]
async fn push_batches() -> Result<()> {
let suite = Suite::default();
let mut fixture = Fixture::default();
let handler = fixture.new_handler();
let Case {
project,
gb_repository,
project_repository,
..
} = &suite.new_case();
} = &fixture.new_case();
suite.sign_in();
fixture.sign_in();
{
let head: git::Oid = project_repository
@ -285,9 +266,7 @@ async fn push_batches() -> Result<()> {
set_test_target(gb_repository, project_repository).unwrap();
let target_id = gb_repository.default_target().unwrap().unwrap().sha;
let reference = project_repository.l(target_id, LogUntil::End).unwrap();
let (cloud_code, _tmp) = test_remote_repository()?;
let api_project = projects::ApiProject {
@ -301,7 +280,7 @@ async fn push_batches() -> Result<()> {
sync: true,
};
suite
fixture
.projects
.update(&projects::UpdateRequest {
id: project.id,
@ -311,15 +290,10 @@ async fn push_batches() -> Result<()> {
.await?;
{
Handler::push_project_to_gitbutler_pure(
suite.local_app_data(),
&suite.projects,
&suite.users,
project.id,
2,
)
.await
.unwrap();
handler
.push_project_to_gitbutler(project.id, 2)
.await
.unwrap();
}
cloud_code.find_commit(target_id.into()).unwrap();
@ -329,7 +303,7 @@ async fn push_batches() -> Result<()> {
assert_eq!(reference, pushed);
assert_eq!(
suite
fixture
.projects
.get(&project.id)
.unwrap()
@ -344,22 +318,20 @@ async fn push_batches() -> Result<()> {
#[tokio::test]
async fn push_again_no_change() -> Result<()> {
let suite = Suite::default();
let mut fixture = Fixture::default();
let handler = fixture.new_handler();
let Case {
project,
gb_repository,
project_repository,
..
} = &suite.new_case_with_files(HashMap::from([(PathBuf::from("test.txt"), "test")]));
} = &fixture.new_case_with_files(HashMap::from([(PathBuf::from("test.txt"), "test")]));
suite.sign_in();
fixture.sign_in();
set_test_target(gb_repository, project_repository).unwrap();
let target_id = gb_repository.default_target().unwrap().unwrap().sha;
let reference = project_repository.l(target_id, LogUntil::End).unwrap();
let (cloud_code, _tmp) = test_remote_repository()?;
let api_project = projects::ApiProject {
@ -373,7 +345,7 @@ async fn push_again_no_change() -> Result<()> {
sync: true,
};
suite
fixture
.projects
.update(&projects::UpdateRequest {
id: project.id,
@ -385,15 +357,10 @@ async fn push_again_no_change() -> Result<()> {
cloud_code.find_commit(target_id.into()).unwrap_err();
{
Handler::push_project_to_gitbutler_pure(
suite.local_app_data(),
&suite.projects,
&suite.users,
project.id,
10,
)
.await
.unwrap();
handler
.push_project_to_gitbutler(project.id, 10)
.await
.unwrap();
}
cloud_code.find_commit(target_id.into()).unwrap();
@ -403,7 +370,7 @@ async fn push_again_no_change() -> Result<()> {
assert_eq!(reference, pushed);
assert_eq!(
suite
fixture
.projects
.get(&project.id)
.unwrap()