From a506b97e71f8e13dee9f4111f9ad732f6415da00 Mon Sep 17 00:00:00 2001 From: Joe Date: Sun, 1 Mar 2026 17:09:35 -0600 Subject: [PATCH] Refactor git repository initialization and enhance logging levels - Updated the `flatten_tree` function in `git.rs` to clarify the repository parameter name. - Changed logging level from `debug` to `trace` in `finalize_patch_application` for more detailed output. - Modified the `TestServeSession` initialization to persist git configuration to disk, ensuring user identity is accessible for subsequent operations. These changes aim to improve code clarity, enhance logging detail, and ensure proper git configuration handling in tests. --- src/git.rs | 2 +- src/snapshot/patch_apply.rs | 2 +- tests/rojo_test/serve_util.rs | 22 ++++++++++------------ tests/tests/git_sync_defaults.rs | 23 ++++++++++++++++------- 4 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/git.rs b/src/git.rs index 8a88b211..ee7b29ae 100644 --- a/src/git.rs +++ b/src/git.rs @@ -212,7 +212,7 @@ fn collect_tree_index_changes( } fn flatten_tree( - _repo: &gix::Repository, + repo: &gix::Repository, tree: &gix::Tree<'_>, prefix: &str, out: &mut HashMap, diff --git a/src/snapshot/patch_apply.rs b/src/snapshot/patch_apply.rs index 8475e67b..150bfd6d 100644 --- a/src/snapshot/patch_apply.rs +++ b/src/snapshot/patch_apply.rs @@ -145,7 +145,7 @@ fn finalize_patch_application(context: PatchApplyContext, tree: &mut RojoTree) - for (id, map) in context.path_refs_to_rewrite { for (prop_name, path) in map { if let Some(target) = tree.resolve_ref_path(&path, id) { - log::debug!( + log::trace!( "Resolved path reference {} -> {} (path: {})", prop_name, target, diff --git a/tests/rojo_test/serve_util.rs b/tests/rojo_test/serve_util.rs index 227f09c5..e1e606db 100644 --- a/tests/rojo_test/serve_util.rs +++ b/tests/rojo_test/serve_util.rs @@ -202,18 +202,16 @@ impl TestServeSession { copy_recursive(&source_path, &project_path) .expect("Couldn't copy project to temporary directory"); - // Initialize git repo and run setup callback - let mut repo = gix::init(&project_path).expect("gix init failed"); - { - let mut config = repo.config_snapshot_mut(); - config - .set_raw_value(&gix::config::tree::User::NAME, "Test") - .unwrap(); - config - .set_raw_value(&gix::config::tree::User::EMAIL, "test@test.com") - .unwrap(); - let _ = config.commit_auto_rollback().unwrap(); - } + // Initialize git repo and persist config to disk so that + // git_add_all_and_commit (which opens a fresh gix handle via + // gix::discover) can read user identity. The gix SnapshotMut + // API is in-memory only and never writes to .git/config. + gix::init(&project_path).expect("gix init failed"); + fs::write( + project_path.join(".git/config"), + "[user]\n\tname = Test\n\temail = test@test.com\n[core]\n\tautocrlf = false\n", + ) + .expect("Failed to write .git/config"); setup(&project_path); diff --git a/tests/tests/git_sync_defaults.rs b/tests/tests/git_sync_defaults.rs index 01506e85..3b797871 100644 --- a/tests/tests/git_sync_defaults.rs +++ b/tests/tests/git_sync_defaults.rs @@ -20,8 +20,17 @@ use librojo::web_api::{InstanceUpdate, WriteRequest}; use rbx_dom_weak::types::{Ref, Variant}; use rbx_dom_weak::{ustr, UstrMap}; +use crate::rojo_test::io_util::SERVE_TESTS_PATH; use crate::rojo_test::serve_util::TestServeSession; +fn fixture_content(rel_path: &str) -> String { + let path = Path::new(SERVE_TESTS_PATH) + .join("git_sync_defaults") + .join(rel_path); + fs::read_to_string(&path) + .unwrap_or_else(|e| panic!("Failed to read fixture {}: {}", path.display(), e)) +} + fn git_commit_all(dir: &Path, msg: &str) { librojo::git::git_add_all_and_commit(dir, msg); } @@ -204,7 +213,7 @@ fn non_script_change_in_changed_ids_but_no_hash() { #[test] fn committed_hash_matches_head_content() { - let original_content = "local ModuleA = {}\nreturn ModuleA\n"; + let original_content = fixture_content("src/ModuleA.luau"); let mut session = TestServeSession::new_with_git("git_sync_defaults", |path| { git_commit_all(path, "initial commit"); @@ -221,7 +230,7 @@ fn committed_hash_matches_head_content() { .get(&module_id) .expect("ModuleA should have committed hashes"); - let expected_hash = compute_blob_sha1(original_content); + let expected_hash = compute_blob_sha1(&original_content); assert!( hashes.contains(&expected_hash), "Committed hash should match SHA1 of original content. Expected: {}, Got: {:?}", @@ -256,7 +265,7 @@ fn staged_hash_included_when_different_from_head() { hashes.len() ); - let head_hash = compute_blob_sha1("local ModuleA = {}\nreturn ModuleA\n"); + let head_hash = compute_blob_sha1(&fixture_content("src/ModuleA.luau")); let staged_hash = compute_blob_sha1("-- staged version"); assert!(hashes.contains(&head_hash), "Should contain HEAD hash"); @@ -334,7 +343,7 @@ fn init_style_script_appears_in_changed_ids() { #[test] fn init_style_script_has_committed_hash() { - let original = "local DirModule = {}\nreturn DirModule\n"; + let original = fixture_content("src/DirModule/init.luau"); let mut session = TestServeSession::new_with_git("git_sync_defaults", |path| { git_commit_all(path, "initial commit"); @@ -350,7 +359,7 @@ fn init_style_script_has_committed_hash() { .get(&dir_id) .expect("DirModule should have committed hashes"); - let expected_hash = compute_blob_sha1(original); + let expected_hash = compute_blob_sha1(&original); assert!( hashes.contains(&expected_hash), "init.luau committed hash should match. Expected: {}, Got: {:?}", @@ -983,7 +992,7 @@ fn committed_changes_still_in_changed_ids_via_initial_head() { #[test] fn committed_script_gets_initial_head_hash() { - let original_content = "local ModuleA = {}\nreturn ModuleA\n"; + let original_content = fixture_content("src/ModuleA.luau"); let modified_content = "-- committed v2\nreturn {}"; let mut session = TestServeSession::new_with_git("git_sync_defaults", |path| { @@ -1009,7 +1018,7 @@ fn committed_script_gets_initial_head_hash() { .get(&module_id) .expect("Committed script should have hashes"); - let initial_hash = compute_blob_sha1(original_content); + let initial_hash = compute_blob_sha1(&original_content); assert!( hashes.contains(&initial_hash), "Hashes should include the initial_head version ({initial_hash}), got {hashes:?}"