diff --git a/src-tauri/src/git/cli.rs b/src-tauri/src/git/cli.rs index d68d1ea..5cfa17c 100644 --- a/src-tauri/src/git/cli.rs +++ b/src-tauri/src/git/cli.rs @@ -318,7 +318,15 @@ impl CliGitHandler { args: &[&str], current_dir: Option<&Path>, ) -> GitResult { - Self::run_git_allow_exit_codes_with_optional_locks(args, current_dir, &[], false) + Self::run_git_allow_exit_codes_without_optional_locks(args, current_dir, &[]) + } + + fn run_git_allow_exit_codes_without_optional_locks( + args: &[&str], + current_dir: Option<&Path>, + extra_ok_codes: &[i32], + ) -> GitResult { + Self::run_git_allow_exit_codes_with_optional_locks(args, current_dir, extra_ok_codes, false) } fn run_git_bytes(args: &[&str], current_dir: Option<&Path>) -> GitResult> { @@ -1031,7 +1039,7 @@ impl CliGitHandler { } fn ensure_clean_for_three_way_patch(repo_path: &Path) -> GitResult<()> { - let output = Self::run_git( + let output = Self::run_git_without_optional_locks( &["status", "--porcelain=v1", "--untracked-files=no"], Some(repo_path), )?; @@ -1060,7 +1068,10 @@ impl CliGitHandler { stderr, exit_code, }) => { - let status_output = Self::run_git(&["status", "--porcelain=v1"], Some(repo_path))?; + let status_output = Self::run_git_without_optional_locks( + &["status", "--porcelain=v1"], + Some(repo_path), + )?; if Self::status_has_unmerged_files(&status_output) { return Ok(Self::operation_result( Self::PATCH_IMPORT_CONFLICTS.to_string(), @@ -1129,7 +1140,7 @@ impl CliGitHandler { fn submodule_is_dirty(repo_path: &Path, path: &str) -> bool { let submodule_path = repo_path.join(path); - Self::run_git_allow_exit_codes( + Self::run_git_allow_exit_codes_without_optional_locks( &["-c", "core.quotepath=false", "status", "--porcelain=v1"], Some(&submodule_path), &[128], @@ -1163,7 +1174,10 @@ impl CliGitHandler { } } - pub(crate) fn collect_submodules_for_status(repo_path: &Path) -> Vec { + pub(crate) fn collect_submodules_for_status( + repo_path: &Path, + dirty_submodule_paths: Option<&HashSet>, + ) -> Vec { let configured = Self::collect_configured_submodules(repo_path).unwrap_or_default(); configured .into_iter() @@ -1178,7 +1192,10 @@ impl CliGitHandler { let uninitialised = status_prefix == Some('-') || (!initialised && expected_commit.is_some() && status_prefix != Some('U')); let missing = !worktree_path.exists() && !uninitialised; - let dirty = initialised && Self::submodule_is_dirty(repo_path, &submodule.path); + let dirty = initialised + && dirty_submodule_paths + .map(|paths| paths.contains(&submodule.path)) + .unwrap_or_else(|| Self::submodule_is_dirty(repo_path, &submodule.path)); let out_of_sync = status_prefix == Some('+') || expected_commit .as_ref() @@ -2808,7 +2825,7 @@ impl GitOperationHandler for CliGitHandler { )?; let mut status = Self::parse_repo_status(&output); Self::refresh_unversioned_items(&mut status, &repo_path); - status.submodules = Self::collect_submodules_for_status(&repo_path); + status.submodules = Self::collect_submodules_for_status(&repo_path, None); Self::remove_submodule_file_entries(&mut status); status.current_branch = Self::detect_current_branch(&repo_path); status.detached_head = status @@ -2856,7 +2873,7 @@ impl GitOperationHandler for CliGitHandler { // Gather numstat for unstaged changes let unstaged_numstat = - Self::run_git_without_optional_locks(&["diff", "--numstat"], Some(&repo_path)) + Self::run_git_without_optional_locks(&["diff-files", "--numstat"], Some(&repo_path)) .unwrap_or_default(); let unstaged_stats = Self::parse_numstat(&unstaged_numstat); @@ -3424,7 +3441,7 @@ impl GitOperationHandler for CliGitHandler { let file_path = request.file_path.trim(); // Check if the file is untracked - let status_output = Self::run_git( + let status_output = Self::run_git_without_optional_locks( &[ "-c", "core.quotepath=false", @@ -5403,7 +5420,7 @@ impl CliGitHandler { } fn is_untracked_file(repo_path: &Path, file_path: &str) -> bool { - let output = match Self::run_git( + let output = match Self::run_git_without_optional_locks( &[ "-c", "core.quotepath=false", @@ -5987,6 +6004,38 @@ impl CliGitHandler { mod tests { use super::*; + fn repo_with_stale_index_entry() -> (tempfile::TempDir, PathBuf, Vec) { + let repo = tempfile::TempDir::new().expect("create temporary repository"); + let run_git = |args: &[&str]| { + let status = Command::new("git") + .args(args) + .current_dir(repo.path()) + .status() + .expect("run git"); + assert!(status.success(), "git {args:?} failed"); + }; + run_git(&["init", "-b", "main"]); + run_git(&["config", "user.email", "test@gitmun.test"]); + run_git(&["config", "user.name", "Gitmun Test"]); + run_git(&["config", "commit.gpgsign", "false"]); + fs::write(repo.path().join("inspection-record.txt"), "stable").expect("write tracked file"); + run_git(&["add", "inspection-record.txt"]); + run_git(&["commit", "-m", "initial"]); + + let index_path = repo.path().join(".git/index"); + let index_before = fs::read(&index_path).expect("read index"); + let tracked_file = fs::OpenOptions::new() + .write(true) + .open(repo.path().join("inspection-record.txt")) + .expect("open tracked file"); + tracked_file + .set_times( + fs::FileTimes::new().set_modified(UNIX_EPOCH + std::time::Duration::from_secs(1)), + ) + .expect("set tracked file timestamp"); + (repo, index_path, index_before) + } + #[test] fn hunk_header_standard() { assert_eq!( @@ -6097,6 +6146,36 @@ UD deleted_by_them.rs assert!(result.is_empty()); } + #[test] + fn three_way_patch_cleanliness_check_does_not_rewrite_index() { + let (repo, index_path, index_before) = repo_with_stale_index_entry(); + + CliGitHandler::ensure_clean_for_three_way_patch(repo.path()) + .expect("check clean repository"); + + assert_eq!( + fs::read(&index_path).expect("read index after cleanliness check"), + index_before + ); + assert!(!index_path.with_file_name("index.lock").exists()); + } + + #[test] + fn untracked_file_check_does_not_rewrite_index() { + let (repo, index_path, index_before) = repo_with_stale_index_entry(); + fs::write(repo.path().join("field-notes.txt"), "untracked").expect("write untracked file"); + + assert!(CliGitHandler::is_untracked_file( + repo.path(), + "field-notes.txt" + )); + assert_eq!( + fs::read(&index_path).expect("read index after untracked file check"), + index_before + ); + assert!(!index_path.with_file_name("index.lock").exists()); + } + #[test] fn repo_status_untracked_file() { let output = "?? new_file.txt\n"; diff --git a/src-tauri/src/git/gix_handler.rs b/src-tauri/src/git/gix_handler.rs index 805ed26..82bfe6e 100644 --- a/src-tauri/src/git/gix_handler.rs +++ b/src-tauri/src/git/gix_handler.rs @@ -143,6 +143,20 @@ impl GixGitHandler { } } + fn conflict_type(conflict: gix::status::plumbing::index_as_worktree::Conflict) -> &'static str { + use gix::status::plumbing::index_as_worktree::Conflict; + + match conflict { + Conflict::BothDeleted => "both_deleted", + Conflict::AddedByUs => "added_by_us", + Conflict::DeletedByThem => "deleted_by_them", + Conflict::AddedByThem => "added_by_them", + Conflict::DeletedByUs => "deleted_by_us", + Conflict::BothAdded => "both_added", + Conflict::BothModified => "both_modified", + } + } + fn current_branch(repo: &gix::Repository) -> Option { match repo.head_name() { Ok(Some(name)) => Some(Self::bstr_to_string(name.shorten())), @@ -728,42 +742,6 @@ impl GixGitHandler { }) } - fn detect_conflicted_files(git_dir: &Path) -> Vec { - let repo_path = git_dir.parent().unwrap_or(git_dir); - let output = crate::configured_git_command() - .args(["-c", "core.quotepath=false", "status", "--porcelain=v1"]) - .current_dir(repo_path) - .output() - .ok() - .and_then(|o| String::from_utf8(o.stdout).ok()) - .unwrap_or_default(); - - let mut conflicts = Vec::new(); - for line in output.lines() { - if line.len() < 4 { - continue; - } - let x = line.as_bytes()[0] as char; - let y = line.as_bytes()[1] as char; - let path = line[3..].to_string(); - let conflict_type = match (x, y) { - ('U', 'U') => "both_modified", - ('A', 'A') => "both_added", - ('D', 'D') => "both_deleted", - ('A', 'U') => "added_by_us", - ('U', 'A') => "added_by_them", - ('D', 'U') => "deleted_by_us", - ('U', 'D') => "deleted_by_them", - _ => continue, - }; - conflicts.push(ConflictFileItem { - path, - conflict_type: conflict_type.to_string(), - }); - } - conflicts - } - fn parse_numstat(output: &str) -> HashMap { let mut stats = HashMap::new(); for line in output.lines().filter(|line| !line.trim().is_empty()) { @@ -782,9 +760,16 @@ impl GixGitHandler { fn collect_numstat(repo_path: &Path, staged: bool) -> HashMap { let mut command = crate::configured_git_command(); command.env("GIT_OPTIONAL_LOCKS", "0"); - command.arg("-c").arg("core.quotepath=false").arg("diff"); if staged { - command.arg("--cached"); + command + .arg("-c") + .arg("core.quotepath=false") + .args(["diff", "--cached"]); + } else { + command + .arg("-c") + .arg("core.quotepath=false") + .arg("diff-files"); } command.arg("--numstat").current_dir(repo_path); @@ -824,6 +809,8 @@ impl GixGitHandler { let mut changed_by_path: HashMap = HashMap::new(); let mut staged_by_path: HashMap = HashMap::new(); let mut unversioned_paths: HashSet = HashSet::new(); + let mut dirty_submodule_paths: HashSet = HashSet::new(); + let mut gix_conflicted_files = Vec::new(); let repo_path = repo.workdir().unwrap_or(repo.path()); let tracked_paths: Vec = repo .index() @@ -839,6 +826,10 @@ impl GixGitHandler { let mut status_iter = repo .status(gix::progress::Discard) .map_err(|error| Self::gix_error(None, error))? + .index_worktree_submodules(gix::status::Submodule::Given { + ignore: gix::submodule::config::Ignore::None, + check_dirty: false, + }) .into_iter(Vec::::new()) .map_err(|error| Self::gix_error(None, error))?; @@ -847,6 +838,34 @@ impl GixGitHandler { match item { gix::status::Item::IndexWorktree(worktree_item) => { + if let gix::status::index_worktree::Item::Modification { + rela_path, + status, + .. + } = &worktree_item + { + use gix::status::plumbing::index_as_worktree::{Change, EntryStatus}; + + match status { + EntryStatus::Conflict { summary, .. } => { + gix_conflicted_files.push(ConflictFileItem { + path: Self::bstr_to_string(rela_path.as_ref()), + conflict_type: Self::conflict_type(*summary).to_string(), + }); + } + EntryStatus::Change(Change::SubmoduleModification(status)) + if status + .changes + .as_ref() + .is_some_and(|changes| !changes.is_empty()) => + { + dirty_submodule_paths + .insert(Self::bstr_to_string(rela_path.as_ref())); + } + _ => {} + } + } + if let gix::status::index_worktree::Item::DirectoryContents { entry, .. } = &worktree_item { @@ -915,6 +934,7 @@ impl GixGitHandler { let mut unversioned_files: Vec = unversioned_paths.into_iter().collect(); unversioned_files.sort(); + gix_conflicted_files.sort_by(|left, right| left.path.cmp(&right.path)); // Detect merge state via filesystem (same approach as CLI handler) let git_dir = repo.git_dir(); @@ -944,7 +964,7 @@ impl GixGitHandler { let conflicted_files = if merge_in_progress || rebase_in_progress || cherry_pick_in_progress { - Self::detect_conflicted_files(git_dir) + gix_conflicted_files } else { vec![] }; @@ -964,7 +984,10 @@ impl GixGitHandler { staged_files, unversioned_files, unversioned_items: vec![], - submodules: CliGitHandler::collect_submodules_for_status(repo_path), + submodules: CliGitHandler::collect_submodules_for_status( + repo_path, + Some(&dirty_submodule_paths), + ), current_branch: Self::current_branch(repo), detached_head: matches!(repo.head_name(), Ok(None)), shallow: CliGitHandler::repo_is_shallow(repo_path), @@ -1550,3 +1573,177 @@ impl GitOperationHandler for GixGitHandler { self.cli_fallback.open_merge_tool(request) } } + +#[cfg(test)] +mod tests { + use super::GixGitHandler; + use crate::git::types::SubmoduleState; + use gix::status::plumbing::index_as_worktree::Conflict; + use std::fs; + use std::path::{Path, PathBuf}; + use std::process::Command; + use tempfile::TempDir; + + fn run_git(repo: &Path, args: &[&str]) { + let status = Command::new("git") + .args(args) + .current_dir(repo) + .status() + .expect("run git"); + assert!(status.success(), "git {args:?} failed"); + } + + fn init_repo() -> TempDir { + let repo = TempDir::new().expect("create temporary repository"); + run_git(repo.path(), &["init", "-b", "main"]); + run_git(repo.path(), &["config", "user.email", "test@gitmun.test"]); + run_git(repo.path(), &["config", "user.name", "Gitmun Test"]); + run_git(repo.path(), &["config", "commit.gpgsign", "false"]); + run_git(repo.path(), &["config", "core.autocrlf", "false"]); + run_git(repo.path(), &["commit", "--allow-empty", "-m", "initial"]); + repo + } + + fn make_index_entry_stale(repo: &Path, file_path: &str) -> (PathBuf, Vec) { + let output = Command::new("git") + .args(["rev-parse", "--git-path", "index"]) + .current_dir(repo) + .output() + .expect("resolve index path"); + assert!(output.status.success(), "resolve index path"); + let raw_index_path = + String::from_utf8(output.stdout).expect("index path should be valid UTF-8"); + let index_path = { + let path = PathBuf::from(raw_index_path.trim()); + if path.is_absolute() { + path + } else { + repo.join(path) + } + }; + let index_before = fs::read(&index_path).expect("read index"); + let tracked_file = fs::OpenOptions::new() + .write(true) + .open(repo.join(file_path)) + .expect("open tracked file"); + tracked_file + .set_times( + fs::FileTimes::new() + .set_modified(std::time::UNIX_EPOCH + std::time::Duration::from_secs(1)), + ) + .expect("set tracked file timestamp"); + (index_path, index_before) + } + + #[test] + fn maps_all_conflict_types() { + let cases = [ + (Conflict::BothDeleted, "both_deleted"), + (Conflict::AddedByUs, "added_by_us"), + (Conflict::DeletedByThem, "deleted_by_them"), + (Conflict::AddedByThem, "added_by_them"), + (Conflict::DeletedByUs, "deleted_by_us"), + (Conflict::BothAdded, "both_added"), + (Conflict::BothModified, "both_modified"), + ]; + + for (conflict, expected) in cases { + assert_eq!(GixGitHandler::conflict_type(conflict), expected); + } + } + + #[test] + fn gix_collector_reads_conflicts_without_rewriting_index() { + let repo_dir = init_repo(); + fs::write(repo_dir.path().join("calibration-report.txt"), "baseline\n") + .expect("write calibration report"); + fs::write(repo_dir.path().join("inspection-record.txt"), "stable\n") + .expect("write inspection record"); + run_git( + repo_dir.path(), + &["add", "calibration-report.txt", "inspection-record.txt"], + ); + run_git( + repo_dir.path(), + &["commit", "-m", "add calibration records"], + ); + run_git(repo_dir.path(), &["switch", "-c", "recalibrate"]); + fs::write( + repo_dir.path().join("calibration-report.txt"), + "branch reading\n", + ) + .expect("write branch reading"); + run_git(repo_dir.path(), &["commit", "-am", "record branch reading"]); + run_git(repo_dir.path(), &["switch", "main"]); + fs::write( + repo_dir.path().join("calibration-report.txt"), + "main reading\n", + ) + .expect("write main reading"); + run_git(repo_dir.path(), &["commit", "-am", "record main reading"]); + + let merge_status = Command::new("git") + .args(["merge", "recalibrate"]) + .current_dir(repo_dir.path()) + .status() + .expect("run git merge"); + assert!(!merge_status.success(), "merge should produce a conflict"); + let (index_path, index_before) = + make_index_entry_stale(repo_dir.path(), "inspection-record.txt"); + + let repo = gix::discover(repo_dir.path()).expect("discover repository"); + let status = + GixGitHandler::collect_repo_status_with_gix(&repo).expect("collect gix status"); + + assert!(status.merge_in_progress); + assert_eq!(status.conflicted_files.len(), 1); + assert_eq!(status.conflicted_files[0].path, "calibration-report.txt"); + assert_eq!(status.conflicted_files[0].conflict_type, "both_modified"); + assert_eq!( + fs::read(&index_path).expect("read index after status"), + index_before + ); + assert!(!index_path.with_file_name("index.lock").exists()); + } + + #[test] + fn gix_collector_reads_dirty_submodule_without_rewriting_index() { + let source = init_repo(); + fs::write(source.path().join("lib.txt"), "v1").expect("write library file"); + run_git(source.path(), &["add", "lib.txt"]); + run_git(source.path(), &["commit", "-m", "add library file"]); + + let parent = init_repo(); + run_git( + parent.path(), + &[ + "-c", + "protocol.file.allow=always", + "submodule", + "add", + source.path().to_str().expect("source path"), + "deps/lib", + ], + ); + run_git(parent.path(), &["commit", "-m", "add submodule"]); + + let submodule_path = parent.path().join("deps/lib"); + let (index_path, index_before) = make_index_entry_stale(&submodule_path, "lib.txt"); + fs::write(submodule_path.join("field-notes.txt"), "untracked") + .expect("write untracked submodule file"); + + let repo = gix::discover(parent.path()).expect("discover repository"); + let status = + GixGitHandler::collect_repo_status_with_gix(&repo).expect("collect gix status"); + + assert_eq!(status.submodules.len(), 1); + assert_eq!(status.submodules[0].path, "deps/lib"); + assert_eq!(status.submodules[0].state, SubmoduleState::Dirty); + assert!(status.submodules[0].dirty); + assert_eq!( + fs::read(&index_path).expect("read submodule index after status"), + index_before + ); + assert!(!index_path.with_file_name("index.lock").exists()); + } +} diff --git a/src-tauri/tests/git.rs b/src-tauri/tests/git.rs index 23814c3..7b9324d 100644 --- a/src-tauri/tests/git.rs +++ b/src-tauri/tests/git.rs @@ -72,6 +72,30 @@ fn write_file(repo: &Path, name: &str, content: &str) { fs::write(repo.join(name), content).expect("write file"); } +fn make_index_entry_stale(repo: &Path, file_path: &str) -> (std::path::PathBuf, Vec) { + let raw_index_path = git_stdout(repo, &["rev-parse", "--git-path", "index"]); + let index_path = { + let path = std::path::PathBuf::from(raw_index_path); + if path.is_absolute() { + path + } else { + repo.join(path) + } + }; + let index_before = fs::read(&index_path).expect("read index"); + let tracked_file = fs::OpenOptions::new() + .write(true) + .open(repo.join(file_path)) + .expect("open tracked file"); + tracked_file + .set_times( + fs::FileTimes::new() + .set_modified(std::time::UNIX_EPOCH + std::time::Duration::from_secs(1)), + ) + .expect("set tracked file timestamp"); + (index_path, index_before) +} + fn read_file(repo: &Path, name: &str) -> String { fs::read_to_string(repo.join(name)).expect("read file") } @@ -310,6 +334,27 @@ fn status_clean_repo() { assert!(status.submodules.is_empty()); } +#[test] +fn status_read_does_not_rewrite_index() { + let dir = init_repo(); + write_file(dir.path(), "inspection-record.txt", "stable"); + git(dir.path(), &["add", "inspection-record.txt"]); + git(dir.path(), &["commit", "-m", "add inspection record"]); + let (index_path, index_before) = make_index_entry_stale(dir.path(), "inspection-record.txt"); + + let status = handler() + .get_repo_status(&repo_request(&dir)) + .expect("get_repo_status"); + + assert!(status.staged_files.is_empty()); + assert!(status.changed_files.is_empty()); + assert_eq!( + fs::read(&index_path).expect("read index after status"), + index_before + ); + assert!(!index_path.with_file_name("index.lock").exists()); +} + #[test] fn status_detects_untracked_file() { let dir = init_repo(); @@ -788,6 +833,10 @@ fn export_commit_patch_rejects_invalid_commit_hash() { #[test] fn discard_file_removes_untracked_directory() { let dir = init_repo(); + write_file(dir.path(), "inspection-record.txt", "stable"); + git(dir.path(), &["add", "inspection-record.txt"]); + git(dir.path(), &["commit", "-m", "add inspection record"]); + let (index_path, index_before) = make_index_entry_stale(dir.path(), "inspection-record.txt"); fs::create_dir_all(dir.path().join("new-dir")).expect("create directory"); write_file(dir.path(), "new-dir/file.txt", "new"); @@ -796,6 +845,11 @@ fn discard_file_removes_untracked_directory() { .expect("discard_file"); assert!(!dir.path().join("new-dir").exists()); + assert_eq!( + fs::read(&index_path).expect("read index after discard"), + index_before + ); + assert!(!index_path.with_file_name("index.lock").exists()); } #[test] @@ -982,19 +1036,26 @@ fn hard_reset_to_head_discards_tracked_changes_and_keeps_untracked_files() { #[test] fn status_detects_clean_submodule() { let (parent, _submodule) = repo_with_submodule(); - let status = handler() - .get_repo_status(&repo_request(&parent)) - .expect("get_repo_status"); + let statuses = [ + handler() + .get_repo_status(&repo_request(&parent)) + .expect("CLI get_repo_status"), + gix_handler() + .get_repo_status(&repo_request(&parent)) + .expect("gix get_repo_status"), + ]; - assert_eq!(status.submodules.len(), 1); - let submodule = &status.submodules[0]; - assert_eq!(submodule.path, "deps/lib"); - assert_eq!(submodule.state, SubmoduleState::Clean); - assert!(submodule.initialised); - assert!(!submodule.dirty); - assert!(!submodule.out_of_sync); - assert!(submodule.expected_commit.is_some()); - assert_eq!(submodule.expected_commit, submodule.checked_out_commit); + for status in statuses { + assert_eq!(status.submodules.len(), 1); + let submodule = &status.submodules[0]; + assert_eq!(submodule.path, "deps/lib"); + assert_eq!(submodule.state, SubmoduleState::Clean); + assert!(submodule.initialised); + assert!(!submodule.dirty); + assert!(!submodule.out_of_sync); + assert!(submodule.expected_commit.is_some()); + assert_eq!(submodule.expected_commit, submodule.checked_out_commit); + } } #[test] @@ -1018,33 +1079,48 @@ fn status_detects_uninitialised_submodule() { #[test] fn status_detects_dirty_submodule_without_normal_file_entry() { let (parent, _submodule) = repo_with_submodule(); - write_file(&parent.path().join("deps/lib"), "lib.txt", "dirty"); + let submodule_path = parent.path().join("deps/lib"); + let (submodule_index, index_before) = make_index_entry_stale(&submodule_path, "lib.txt"); + write_file(&submodule_path, "field-notes.txt", "dirty"); - let status = handler() - .get_repo_status(&repo_request(&parent)) - .expect("get_repo_status"); + let statuses = [ + handler() + .get_repo_status(&repo_request(&parent)) + .expect("CLI get_repo_status"), + gix_handler() + .get_repo_status(&repo_request(&parent)) + .expect("gix get_repo_status"), + ]; - let submodule = &status.submodules[0]; - assert_eq!(submodule.state, SubmoduleState::Dirty); - assert!(submodule.dirty); - assert!( - status - .changed_files - .iter() - .all(|file| file.path != "deps/lib") - ); - assert!( - status - .staged_files - .iter() - .all(|file| file.path != "deps/lib") - ); - assert!( - status - .unversioned_files - .iter() - .all(|path| path != "deps/lib") + for status in statuses { + let submodule = &status.submodules[0]; + assert_eq!(submodule.state, SubmoduleState::Dirty); + assert!(submodule.dirty); + assert!( + status + .changed_files + .iter() + .all(|file| file.path != "deps/lib") + ); + assert!( + status + .staged_files + .iter() + .all(|file| file.path != "deps/lib") + ); + assert!( + status + .unversioned_files + .iter() + .all(|path| path != "deps/lib") + ); + } + + assert_eq!( + fs::read(&submodule_index).expect("read submodule index after status"), + index_before ); + assert!(!submodule_index.with_file_name("index.lock").exists()); } #[test] @@ -1061,20 +1137,60 @@ fn status_detects_out_of_sync_submodule() { git(&submodule_path, &["add", "lib.txt"]); git(&submodule_path, &["commit", "-m", "advance submodule"]); - let status = handler() + let statuses = [ + handler() + .get_repo_status(&repo_request(&parent)) + .expect("CLI get_repo_status"), + gix_handler() + .get_repo_status(&repo_request(&parent)) + .expect("gix get_repo_status"), + ]; + + for status in statuses { + let submodule = &status.submodules[0]; + assert_eq!(submodule.state, SubmoduleState::OutOfSync); + assert!(submodule.out_of_sync); + assert!(!submodule.dirty); + assert_ne!(submodule.expected_commit, submodule.checked_out_commit); + assert!( + status + .changed_files + .iter() + .all(|file| file.path != "deps/lib") + ); + } +} + +#[test] +fn gix_status_detects_staged_submodule_change() { + let (parent, _submodule) = repo_with_submodule(); + let submodule_path = parent.path().join("deps/lib"); + write_file(&submodule_path, "lib.txt", "staged"); + git(&submodule_path, &["add", "lib.txt"]); + + let status = gix_handler() .get_repo_status(&repo_request(&parent)) - .expect("get_repo_status"); + .expect("gix get_repo_status"); - let submodule = &status.submodules[0]; - assert_eq!(submodule.state, SubmoduleState::OutOfSync); - assert!(submodule.out_of_sync); - assert_ne!(submodule.expected_commit, submodule.checked_out_commit); - assert!( - status - .changed_files - .iter() - .all(|file| file.path != "deps/lib") + assert_eq!(status.submodules[0].state, SubmoduleState::Dirty); + assert!(status.submodules[0].dirty); +} + +#[test] +fn gix_status_detects_untracked_submodule_change() { + let (parent, _submodule) = repo_with_submodule(); + write_file( + &parent.path().join("deps/lib"), + "local-notes.txt", + "untracked", ); + + let status = gix_handler() + .get_repo_status(&repo_request(&parent)) + .expect("gix get_repo_status"); + + assert_eq!(status.submodules[0].state, SubmoduleState::Dirty); + assert!(status.submodules[0].dirty); } #[test] diff --git a/src/components/centre/CommitBox.test.tsx b/src/components/centre/CommitBox.test.tsx index 2069440..6c560c0 100644 --- a/src/components/centre/CommitBox.test.tsx +++ b/src/components/centre/CommitBox.test.tsx @@ -1,6 +1,6 @@ // @vitest-environment jsdom import React from "react"; -import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { CommitBox } from "./CommitBox"; import type { CommitPrimaryAction } from "../../types"; @@ -50,7 +50,7 @@ function renderCommitBox({ mergeMessage, mergeInProgress, }: RenderCommitBoxOptions = {}) { - const onCommit = vi.fn(); + const onCommit = vi.fn(() => false); const onSelectAction = vi.fn(); const view = render( @@ -79,7 +79,7 @@ describe("CommitBox", () => { vi.stubGlobal("localStorage", makeLocalStorage()); localStorage.removeItem(COMMIT_BOX_RATIO_KEY); getCommitMessageRecovery.mockClear(); - getCommitMessageRecovery.mockResolvedValue(null); + getCommitMessageRecovery.mockReturnValue(new Promise(() => {})); }); afterEach(() => { @@ -259,16 +259,20 @@ describe("CommitBox", () => { }); it("does not offer COMMIT_EDITMSG recovery for the latest commit message", async () => { - getCommitMessageRecovery.mockResolvedValue({ + const recovery = Promise.resolve({ message: "Publish tide report\n\nNo new recovery copy.", updatedAt: 1, }); + getCommitMessageRecovery.mockReturnValue(recovery); renderCommitBox({ repoPath, lastCommitMessage: "Publish tide report\n\nNo new recovery copy.", }); - await waitFor(() => expect(getCommitMessageRecovery).toHaveBeenCalled()); + await act(async () => { + await recovery; + }); + expect(getCommitMessageRecovery).toHaveBeenCalled(); expect(screen.queryByRole("button", {name: "Restore previous message"})).not.toBeInTheDocument(); }); diff --git a/src/components/centre/StagingView.test.tsx b/src/components/centre/StagingView.test.tsx index b102a25..f85e3c6 100644 --- a/src/components/centre/StagingView.test.tsx +++ b/src/components/centre/StagingView.test.tsx @@ -7,7 +7,7 @@ import "../../i18n"; import { StagingView } from "./StagingView"; vi.mock("../../api/commands", () => ({ - getCommitMessageRecovery: vi.fn(async () => null), + getCommitMessageRecovery: vi.fn(() => new Promise(() => {})), getNumstat: vi.fn(), })); diff --git a/src/components/settings/SettingsWindow.test.tsx b/src/components/settings/SettingsWindow.test.tsx index c40fd46..60e6b33 100644 --- a/src/components/settings/SettingsWindow.test.tsx +++ b/src/components/settings/SettingsWindow.test.tsx @@ -1,13 +1,14 @@ // @vitest-environment jsdom import React from "react"; import {fireEvent, render, screen, waitFor} from "@testing-library/react"; -import {beforeEach, describe, expect, it, vi} from "vitest"; +import {afterEach, beforeEach, describe, expect, it, vi} from "vitest"; import type {Settings} from "../../types"; import "../../i18n"; const mocks = vi.hoisted(() => ({ close: vi.fn(), emit: vi.fn(async () => {}), + getAppUpdateChannel: vi.fn(async () => "SystemManaged"), openDialog: vi.fn(), openPath: vi.fn(), invoke: vi.fn(), @@ -88,7 +89,7 @@ vi.mock("@tauri-apps/plugin-dialog", () => ({open: mocks.openDialog})); vi.mock("@tauri-apps/plugin-opener", () => ({openPath: mocks.openPath})); vi.mock("@tauri-apps/plugin-os", () => ({platform: () => "linux"})); vi.mock("../../api/commands", () => ({ - getAppUpdateChannel: vi.fn(async () => "SystemManaged"), + getAppUpdateChannel: mocks.getAppUpdateChannel, getConfigFilePath: vi.fn(async () => "/home/conor/.config/gitmun/config.toml"), getConfigFolderPath: vi.fn(async () => "/home/conor/.config/gitmun"), getGlobalDiffToolPath: vi.fn(async () => null), @@ -111,6 +112,8 @@ describe("SettingsWindow", () => { mocks.invoke.mockClear(); mocks.invoke.mockImplementation(defaultInvoke); mocks.emit.mockClear(); + mocks.getAppUpdateChannel.mockClear(); + mocks.getAppUpdateChannel.mockResolvedValue("SystemManaged"); mocks.close.mockClear(); const store = new Map(); vi.stubGlobal("localStorage", { @@ -135,14 +138,12 @@ describe("SettingsWindow", () => { }); }); - it("shows a skeleton while settings are loading", () => { - mocks.invoke.mockImplementation((command: string) => { - if (command === "get_settings") { - return new Promise(() => {}); - } + afterEach(() => { + vi.restoreAllMocks(); + }); - return defaultInvoke(command); - }); + it("shows a skeleton while settings are loading", () => { + mocks.getAppUpdateChannel.mockReturnValue(new Promise(() => {})); render(); @@ -178,9 +179,11 @@ describe("SettingsWindow", () => { }); it("shows an error when settings fail to load", async () => { + const loadError = new Error("config unavailable"); + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); mocks.invoke.mockImplementation((command: string) => { if (command === "get_settings") { - return Promise.reject(new Error("config unavailable")); + return Promise.reject(loadError); } return defaultInvoke(command); @@ -191,15 +194,18 @@ describe("SettingsWindow", () => { expect(await screen.findByRole("alert")).toHaveTextContent("Settings could not be loaded"); expect(screen.getByRole("button", {name: "Retry"})).toBeInTheDocument(); expect(screen.queryByTestId("settings-skeleton")).not.toBeInTheDocument(); + expect(consoleError).toHaveBeenCalledWith("Failed to load settings", loadError); }); it("retries loading settings when retry is clicked", async () => { + const loadError = new Error("config unavailable"); + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); let getSettingsCalls = 0; mocks.invoke.mockImplementation((command: string) => { if (command === "get_settings") { getSettingsCalls += 1; if (getSettingsCalls === 1) { - return Promise.reject(new Error("config unavailable")); + return Promise.reject(loadError); } } @@ -213,6 +219,7 @@ describe("SettingsWindow", () => { expect(await screen.findByLabelText("Terminal")).toBeInTheDocument(); expect(screen.queryByRole("alert")).not.toBeInTheDocument(); expect(getSettingsCalls).toBe(2); + expect(consoleError).toHaveBeenCalledWith("Failed to load settings", loadError); }); it("shows the Linux custom terminal command only for Custom", async () => {