mirror of
https://github.com/rojo-rbx/rojo.git
synced 2026-10-23 05:51:39 +00:00
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.
This commit is contained in:
parent
095e78dff2
commit
434a3c458a
5 changed files with 81 additions and 43 deletions
|
|
@ -4,28 +4,23 @@ Run the complete CI pipeline by executing the appropriate platform script.
|
||||||
|
|
||||||
## Instructions
|
## Instructions
|
||||||
|
|
||||||
### 1. Determine rbx-dom Flag
|
### 1. Run the CI Script
|
||||||
|
|
||||||
- By default, run **without** the skip flag (full suite including rbx-dom).
|
**Always run the full suite including rbx-dom. No exceptions. No flags.**
|
||||||
- If the user's message asks to **skip rbx-dom**, pass the skip flag (`-NoRbxDom` / `--no-rbx-dom`).
|
|
||||||
|
|
||||||
### 2. Run the CI Script
|
|
||||||
|
|
||||||
**Windows (PowerShell):**
|
**Windows (PowerShell):**
|
||||||
```powershell
|
```powershell
|
||||||
.\scripts\ci.ps1 # with rbx-dom
|
.\scripts\ci.ps1
|
||||||
.\scripts\ci.ps1 -NoRbxDom # without rbx-dom
|
|
||||||
```
|
```
|
||||||
|
|
||||||
**macOS/Linux (Bash):**
|
**macOS/Linux (Bash):**
|
||||||
```bash
|
```bash
|
||||||
bash scripts/ci.sh # with rbx-dom
|
bash scripts/ci.sh
|
||||||
bash scripts/ci.sh --no-rbx-dom # without rbx-dom
|
|
||||||
```
|
```
|
||||||
|
|
||||||
**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:
|
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.
|
- **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 <test_name>`, `cargo test -p <crate>`). Do not re-run the entire suite until you believe all issues are resolved.
|
- **Re-run only the failing test(s)** to verify fixes quickly (e.g. `cargo test <test_name>`, `cargo test -p <crate>`). 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. 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.
|
Once CI passes clean, provide a brief summary of what was fixed (if anything) and confirm the final result.
|
||||||
|
|
|
||||||
|
|
@ -811,17 +811,23 @@ impl JobThreadContext {
|
||||||
let deduped_new_name;
|
let deduped_new_name;
|
||||||
{
|
{
|
||||||
let candidate_file = if extension.is_empty() {
|
let candidate_file = if extension.is_empty() {
|
||||||
format!("{}{}", slugified_new_name, script_suffix)
|
format!(
|
||||||
|
"{}{}",
|
||||||
|
slugified_new_name, script_suffix
|
||||||
|
)
|
||||||
} else {
|
} else {
|
||||||
format!(
|
format!(
|
||||||
"{}{}.{}",
|
"{}{}.{}",
|
||||||
slugified_new_name, script_suffix, extension
|
slugified_new_name,
|
||||||
|
script_suffix,
|
||||||
|
extension
|
||||||
)
|
)
|
||||||
};
|
};
|
||||||
let candidate = parent.join(&candidate_file);
|
let candidate = parent.join(&candidate_file);
|
||||||
if candidate != *path && candidate.exists() {
|
if candidate != *path && candidate.exists() {
|
||||||
let mut taken: std::collections::HashSet<String> =
|
let mut taken: std::collections::HashSet<
|
||||||
std::collections::HashSet::new();
|
String,
|
||||||
|
> = std::collections::HashSet::new();
|
||||||
if let Ok(entries) = fs::read_dir(parent) {
|
if let Ok(entries) = fs::read_dir(parent) {
|
||||||
for entry in entries.flatten() {
|
for entry in entries.flatten() {
|
||||||
let ep = entry.path();
|
let ep = entry.path();
|
||||||
|
|
@ -835,15 +841,18 @@ impl JobThreadContext {
|
||||||
.file_stem()
|
.file_stem()
|
||||||
.and_then(|f| f.to_str())
|
.and_then(|f| f.to_str())
|
||||||
.unwrap_or("");
|
.unwrap_or("");
|
||||||
strip_script_suffix(s).to_lowercase()
|
strip_script_suffix(s)
|
||||||
|
.to_lowercase()
|
||||||
};
|
};
|
||||||
taken.insert(slug);
|
taken.insert(slug);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// Free the slot we're vacating
|
// Free the slot we're vacating
|
||||||
taken.remove(&old_base.to_lowercase());
|
taken.remove(&old_base.to_lowercase());
|
||||||
deduped_new_name =
|
deduped_new_name = deduplicate_name(
|
||||||
deduplicate_name(&slugified_new_name, &taken);
|
&slugified_new_name,
|
||||||
|
&taken,
|
||||||
|
);
|
||||||
} else {
|
} else {
|
||||||
deduped_new_name = slugified_new_name.clone();
|
deduped_new_name = slugified_new_name.clone();
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -21,8 +21,8 @@ use crate::{
|
||||||
},
|
},
|
||||||
snapshot_middleware::Middleware,
|
snapshot_middleware::Middleware,
|
||||||
syncback::{
|
syncback::{
|
||||||
filter_properties, inst_path, name_needs_slugify, slugify_name,
|
filter_properties, inst_path, name_needs_slugify, slugify_name, strip_middleware_extension,
|
||||||
strip_middleware_extension, FsSnapshot, SyncbackReturn, SyncbackSnapshot,
|
FsSnapshot, SyncbackReturn, SyncbackSnapshot,
|
||||||
},
|
},
|
||||||
variant_eq::variant_eq,
|
variant_eq::variant_eq,
|
||||||
RojoRef,
|
RojoRef,
|
||||||
|
|
|
||||||
|
|
@ -2407,13 +2407,19 @@ mod tests {
|
||||||
// (filename, needs_meta, dedup_key) via name_for_inst
|
// (filename, needs_meta, dedup_key) via name_for_inst
|
||||||
let mut dom = rbx_dom_weak::WeakDom::new(rbx_dom_weak::InstanceBuilder::new("Folder"));
|
let mut dom = rbx_dom_weak::WeakDom::new(rbx_dom_weak::InstanceBuilder::new("Folder"));
|
||||||
let root_ref = dom.root_ref();
|
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 inst = dom.get_by_ref(child).unwrap();
|
||||||
let taken = std::collections::HashSet::new();
|
let taken = std::collections::HashSet::new();
|
||||||
let (filename, needs_meta, dedup_key) = name_for_inst(
|
let (filename, needs_meta, dedup_key) = name_for_inst(
|
||||||
crate::snapshot_middleware::Middleware::ModuleScript,
|
crate::snapshot_middleware::Middleware::ModuleScript,
|
||||||
inst, None, &taken,
|
inst,
|
||||||
).unwrap();
|
None,
|
||||||
|
&taken,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
assert_eq!(filename.as_ref(), "Hey_Bro.luau");
|
assert_eq!(filename.as_ref(), "Hey_Bro.luau");
|
||||||
assert!(needs_meta);
|
assert!(needs_meta);
|
||||||
assert_eq!(dedup_key, "Hey_Bro");
|
assert_eq!(dedup_key, "Hey_Bro");
|
||||||
|
|
@ -2423,21 +2429,33 @@ mod tests {
|
||||||
fn syncback_collision_deduplicates() {
|
fn syncback_collision_deduplicates() {
|
||||||
let mut dom = rbx_dom_weak::WeakDom::new(rbx_dom_weak::InstanceBuilder::new("Folder"));
|
let mut dom = rbx_dom_weak::WeakDom::new(rbx_dom_weak::InstanceBuilder::new("Folder"));
|
||||||
let root_ref = dom.root_ref();
|
let root_ref = dom.root_ref();
|
||||||
let child1 = dom.insert(root_ref, rbx_dom_weak::InstanceBuilder::new("ModuleScript").with_name("A/B"));
|
let child1 = dom.insert(
|
||||||
let child2 = dom.insert(root_ref, rbx_dom_weak::InstanceBuilder::new("ModuleScript").with_name("A:B"));
|
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 inst1 = dom.get_by_ref(child1).unwrap();
|
||||||
let inst2 = dom.get_by_ref(child2).unwrap();
|
let inst2 = dom.get_by_ref(child2).unwrap();
|
||||||
|
|
||||||
let mut taken = std::collections::HashSet::new();
|
let mut taken = std::collections::HashSet::new();
|
||||||
let (f1, m1, dk1) = name_for_inst(
|
let (f1, m1, dk1) = name_for_inst(
|
||||||
crate::snapshot_middleware::Middleware::ModuleScript,
|
crate::snapshot_middleware::Middleware::ModuleScript,
|
||||||
inst1, None, &taken,
|
inst1,
|
||||||
).unwrap();
|
None,
|
||||||
|
&taken,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
taken.insert(dk1.to_lowercase());
|
taken.insert(dk1.to_lowercase());
|
||||||
let (f2, m2, dk2) = name_for_inst(
|
let (f2, m2, dk2) = name_for_inst(
|
||||||
crate::snapshot_middleware::Middleware::ModuleScript,
|
crate::snapshot_middleware::Middleware::ModuleScript,
|
||||||
inst2, None, &taken,
|
inst2,
|
||||||
).unwrap();
|
None,
|
||||||
|
&taken,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
assert_eq!(f1.as_ref(), "A_B.luau");
|
assert_eq!(f1.as_ref(), "A_B.luau");
|
||||||
assert!(m1);
|
assert!(m1);
|
||||||
|
|
@ -2455,22 +2473,35 @@ mod tests {
|
||||||
|
|
||||||
let mut dom = rbx_dom_weak::WeakDom::new(rbx_dom_weak::InstanceBuilder::new("Folder"));
|
let mut dom = rbx_dom_weak::WeakDom::new(rbx_dom_weak::InstanceBuilder::new("Folder"));
|
||||||
let root = dom.root_ref();
|
let root = dom.root_ref();
|
||||||
let refs: Vec<_> = names.iter().map(|n|
|
let refs: Vec<_> = names
|
||||||
dom.insert(root, rbx_dom_weak::InstanceBuilder::new("ModuleScript").with_name(*n))
|
.iter()
|
||||||
).collect();
|
.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();
|
let mut taken = std::collections::HashSet::new();
|
||||||
refs.iter().map(|r| {
|
refs.iter()
|
||||||
let inst = dom.get_by_ref(*r).unwrap();
|
.map(|r| {
|
||||||
let (f, m, dk) = name_for_inst(mw, inst, None, &taken).unwrap();
|
let inst = dom.get_by_ref(*r).unwrap();
|
||||||
taken.insert(dk.to_lowercase());
|
let (f, m, dk) = name_for_inst(mw, inst, None, &taken).unwrap();
|
||||||
(f.into_owned(), m, dk)
|
taken.insert(dk.to_lowercase());
|
||||||
}).collect()
|
(f.into_owned(), m, dk)
|
||||||
|
})
|
||||||
|
.collect()
|
||||||
};
|
};
|
||||||
|
|
||||||
let result1 = run(&dom, &refs);
|
let result1 = run(&dom, &refs);
|
||||||
let result2 = 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"
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -5226,7 +5226,10 @@ fn rename_slugified_to_clean() {
|
||||||
|
|
||||||
poll_file_exists(&new_path, "CleanName.luau after rename");
|
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_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
|
// The new meta file should either not exist (clean name needs no
|
||||||
// name override) or, if it exists, should NOT contain a "name" field.
|
// name override) or, if it exists, should NOT contain a "name" field.
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue