From 434a3c458a6a12f826971a2135b481b606a32a50 Mon Sep 17 00:00:00 2001 From: Joe Date: Tue, 10 Feb 2026 21:44:29 -0600 Subject: [PATCH] Refactor CI documentation and improve change processing logic - Updated CI documentation to enforce running the full suite without skipping the rbx-dom section, clarifying the execution steps. - Refactored `change_processor.rs` for improved readability by enhancing formatting and structure in name deduplication logic. - Cleaned up imports in `project.rs` and improved formatting in `file_names.rs` to enhance code clarity and maintainability. - Adjusted test assertions in `two_way_sync.rs` for better readability and consistency in checking file existence after renaming operations. --- .cursor/commands/ci.md | 21 ++++----- src/change_processor.rs | 23 +++++++--- src/snapshot_middleware/project.rs | 4 +- src/syncback/file_names.rs | 71 +++++++++++++++++++++--------- tests/tests/two_way_sync.rs | 5 ++- 5 files changed, 81 insertions(+), 43 deletions(-) diff --git a/.cursor/commands/ci.md b/.cursor/commands/ci.md index b1405617..e3f09e0f 100644 --- a/.cursor/commands/ci.md +++ b/.cursor/commands/ci.md @@ -4,28 +4,23 @@ Run the complete CI pipeline by executing the appropriate platform script. ## Instructions -### 1. Determine rbx-dom Flag +### 1. Run the CI Script -- By default, run **without** the skip flag (full suite including rbx-dom). -- If the user's message asks to **skip rbx-dom**, pass the skip flag (`-NoRbxDom` / `--no-rbx-dom`). - -### 2. Run the CI Script +**Always run the full suite including rbx-dom. No exceptions. No flags.** **Windows (PowerShell):** ```powershell -.\scripts\ci.ps1 # with rbx-dom -.\scripts\ci.ps1 -NoRbxDom # without rbx-dom +.\scripts\ci.ps1 ``` **macOS/Linux (Bash):** ```bash -bash scripts/ci.sh # with rbx-dom -bash scripts/ci.sh --no-rbx-dom # without rbx-dom +bash scripts/ci.sh ``` -**Monitor the script output as it runs.** After each step completes, check its result. If a step failed, kill the script and switch to manual fixing (step 3) — don't let it continue to the next step. +**Monitor the script output as it runs.** After each step completes, check its result. If a step failed, kill the script and switch to manual fixing (step 2) — don't let it continue to the next step. -### 3. Fix Failures Between Steps +### 2. Fix Failures Between Steps When a step fails, stop the script and fix the issue before continuing. Use targeted commands to speed up the feedback loop: @@ -41,10 +36,10 @@ When a step fails, stop the script and fix the issue before continuing. Use targ - **Never** delete or gut a test just to make CI green. Tests exist for a reason — the code must satisfy them, not the other way around. - **Re-run only the failing test(s)** to verify fixes quickly (e.g. `cargo test `, `cargo test -p `). Do not re-run the entire suite until you believe all issues are resolved. -3. **After all known issues are fixed**, re-run the full CI script from step 2 to catch any remaining failures. +3. **After all known issues are fixed**, re-run the full CI script from step 1 to catch any remaining failures. 4. Repeat until the script exits with `Overall: PASS`. -### 4. Report +### 3. Report Once CI passes clean, provide a brief summary of what was fixed (if anything) and confirm the final result. diff --git a/src/change_processor.rs b/src/change_processor.rs index 251fc617..784415d5 100644 --- a/src/change_processor.rs +++ b/src/change_processor.rs @@ -811,17 +811,23 @@ impl JobThreadContext { let deduped_new_name; { let candidate_file = if extension.is_empty() { - format!("{}{}", slugified_new_name, script_suffix) + format!( + "{}{}", + slugified_new_name, script_suffix + ) } else { format!( "{}{}.{}", - slugified_new_name, script_suffix, extension + slugified_new_name, + script_suffix, + extension ) }; let candidate = parent.join(&candidate_file); if candidate != *path && candidate.exists() { - let mut taken: std::collections::HashSet = - std::collections::HashSet::new(); + let mut taken: std::collections::HashSet< + String, + > = std::collections::HashSet::new(); if let Ok(entries) = fs::read_dir(parent) { for entry in entries.flatten() { let ep = entry.path(); @@ -835,15 +841,18 @@ impl JobThreadContext { .file_stem() .and_then(|f| f.to_str()) .unwrap_or(""); - strip_script_suffix(s).to_lowercase() + strip_script_suffix(s) + .to_lowercase() }; taken.insert(slug); } } // Free the slot we're vacating taken.remove(&old_base.to_lowercase()); - deduped_new_name = - deduplicate_name(&slugified_new_name, &taken); + deduped_new_name = deduplicate_name( + &slugified_new_name, + &taken, + ); } else { deduped_new_name = slugified_new_name.clone(); } diff --git a/src/snapshot_middleware/project.rs b/src/snapshot_middleware/project.rs index 483c04c7..e73f2719 100644 --- a/src/snapshot_middleware/project.rs +++ b/src/snapshot_middleware/project.rs @@ -21,8 +21,8 @@ use crate::{ }, snapshot_middleware::Middleware, syncback::{ - filter_properties, inst_path, name_needs_slugify, slugify_name, - strip_middleware_extension, FsSnapshot, SyncbackReturn, SyncbackSnapshot, + filter_properties, inst_path, name_needs_slugify, slugify_name, strip_middleware_extension, + FsSnapshot, SyncbackReturn, SyncbackSnapshot, }, variant_eq::variant_eq, RojoRef, diff --git a/src/syncback/file_names.rs b/src/syncback/file_names.rs index 9eaf45d1..fe491bf5 100644 --- a/src/syncback/file_names.rs +++ b/src/syncback/file_names.rs @@ -2407,13 +2407,19 @@ mod tests { // (filename, needs_meta, dedup_key) via name_for_inst let mut dom = rbx_dom_weak::WeakDom::new(rbx_dom_weak::InstanceBuilder::new("Folder")); let root_ref = dom.root_ref(); - let child = dom.insert(root_ref, rbx_dom_weak::InstanceBuilder::new("ModuleScript").with_name("Hey/Bro")); + let child = dom.insert( + root_ref, + rbx_dom_weak::InstanceBuilder::new("ModuleScript").with_name("Hey/Bro"), + ); let inst = dom.get_by_ref(child).unwrap(); let taken = std::collections::HashSet::new(); let (filename, needs_meta, dedup_key) = name_for_inst( crate::snapshot_middleware::Middleware::ModuleScript, - inst, None, &taken, - ).unwrap(); + inst, + None, + &taken, + ) + .unwrap(); assert_eq!(filename.as_ref(), "Hey_Bro.luau"); assert!(needs_meta); assert_eq!(dedup_key, "Hey_Bro"); @@ -2423,21 +2429,33 @@ mod tests { fn syncback_collision_deduplicates() { let mut dom = rbx_dom_weak::WeakDom::new(rbx_dom_weak::InstanceBuilder::new("Folder")); let root_ref = dom.root_ref(); - let child1 = dom.insert(root_ref, rbx_dom_weak::InstanceBuilder::new("ModuleScript").with_name("A/B")); - let child2 = dom.insert(root_ref, rbx_dom_weak::InstanceBuilder::new("ModuleScript").with_name("A:B")); + let child1 = dom.insert( + root_ref, + rbx_dom_weak::InstanceBuilder::new("ModuleScript").with_name("A/B"), + ); + let child2 = dom.insert( + root_ref, + rbx_dom_weak::InstanceBuilder::new("ModuleScript").with_name("A:B"), + ); let inst1 = dom.get_by_ref(child1).unwrap(); let inst2 = dom.get_by_ref(child2).unwrap(); let mut taken = std::collections::HashSet::new(); let (f1, m1, dk1) = name_for_inst( crate::snapshot_middleware::Middleware::ModuleScript, - inst1, None, &taken, - ).unwrap(); + inst1, + None, + &taken, + ) + .unwrap(); taken.insert(dk1.to_lowercase()); let (f2, m2, dk2) = name_for_inst( crate::snapshot_middleware::Middleware::ModuleScript, - inst2, None, &taken, - ).unwrap(); + inst2, + None, + &taken, + ) + .unwrap(); assert_eq!(f1.as_ref(), "A_B.luau"); assert!(m1); @@ -2455,22 +2473,35 @@ mod tests { let mut dom = rbx_dom_weak::WeakDom::new(rbx_dom_weak::InstanceBuilder::new("Folder")); let root = dom.root_ref(); - let refs: Vec<_> = names.iter().map(|n| - dom.insert(root, rbx_dom_weak::InstanceBuilder::new("ModuleScript").with_name(*n)) - ).collect(); + let refs: Vec<_> = names + .iter() + .map(|n| { + dom.insert( + root, + rbx_dom_weak::InstanceBuilder::new("ModuleScript").with_name(*n), + ) + }) + .collect(); - let run = |dom: &rbx_dom_weak::WeakDom, refs: &[rbx_dom_weak::types::Ref]| -> Vec<(String, bool, String)> { + let run = |dom: &rbx_dom_weak::WeakDom, + refs: &[rbx_dom_weak::types::Ref]| + -> Vec<(String, bool, String)> { let mut taken = std::collections::HashSet::new(); - refs.iter().map(|r| { - let inst = dom.get_by_ref(*r).unwrap(); - let (f, m, dk) = name_for_inst(mw, inst, None, &taken).unwrap(); - taken.insert(dk.to_lowercase()); - (f.into_owned(), m, dk) - }).collect() + refs.iter() + .map(|r| { + let inst = dom.get_by_ref(*r).unwrap(); + let (f, m, dk) = name_for_inst(mw, inst, None, &taken).unwrap(); + taken.insert(dk.to_lowercase()); + (f.into_owned(), m, dk) + }) + .collect() }; let result1 = run(&dom, &refs); let result2 = run(&dom, &refs); - assert_eq!(result1, result2, "Two runs of name_for_inst must produce identical output"); + assert_eq!( + result1, result2, + "Two runs of name_for_inst must produce identical output" + ); } } diff --git a/tests/tests/two_way_sync.rs b/tests/tests/two_way_sync.rs index 832ed6f0..b549b0a9 100644 --- a/tests/tests/two_way_sync.rs +++ b/tests/tests/two_way_sync.rs @@ -5226,7 +5226,10 @@ fn rename_slugified_to_clean() { poll_file_exists(&new_path, "CleanName.luau after rename"); poll_not_exists(&old_path, "What_Module.luau should be gone after rename"); - poll_not_exists(&old_meta, "What_Module.meta.json5 should be gone after rename"); + poll_not_exists( + &old_meta, + "What_Module.meta.json5 should be gone after rename", + ); // The new meta file should either not exist (clean name needs no // name override) or, if it exists, should NOT contain a "name" field.