Skip to content

Commit

Permalink
Fix move_folders_of_stuff (rojo-rbx#826)
Browse files Browse the repository at this point in the history
This is a fairly important test verifying whether the action of moving a
folder into a watched folder is correctly detected and processed. It was
disabled in
rojo-rbx@b43b45b.
The fact that it failed indicates a possible bug in change processing,
so in this PR, I'll re-enable the test, investigate why it fails, and
fix it.
  • Loading branch information
kennethloeffler authored Dec 31, 2023
1 parent 11fa08e commit 097d39e
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 48 deletions.
81 changes: 34 additions & 47 deletions src/change_processor.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::{
fs,
path::PathBuf,
sync::{Arc, Mutex},
};

Expand Down Expand Up @@ -126,14 +125,41 @@ impl JobThreadContext {
// For a given VFS event, we might have many changes to different parts
// of the tree. Calculate and apply all of these changes.
let applied_patches = match event {
VfsEvent::Write(path) => {
if path.is_dir() {
return;
VfsEvent::Create(path) | VfsEvent::Remove(path) | VfsEvent::Write(path) => {
let mut tree = self.tree.lock().unwrap();
let mut applied_patches = Vec::new();

// Find the nearest ancestor to this path that has
// associated instances in the tree. This helps make sure
// that we handle additions correctly, especially if we
// receive events for descendants of a large tree being
// created all at once.
let mut current_path = path.as_path();
let affected_ids = loop {
let ids = tree.get_ids_at_path(&current_path);

log::trace!("Path {} affects IDs {:?}", current_path.display(), ids);

if !ids.is_empty() {
break ids.to_vec();
}

log::trace!("Trying parent path...");
match current_path.parent() {
Some(parent) => current_path = parent,
None => break Vec::new(),
}
};

for id in affected_ids {
if let Some(patch) = compute_and_apply_changes(&mut tree, &self.vfs, id) {
if !patch.is_empty() {
applied_patches.push(patch);
}
}
}
on_vfs_event(path, &self.tree, &self.vfs)
}
VfsEvent::Create(path) | VfsEvent::Remove(path) => {
on_vfs_event(path, &self.tree, &self.vfs)

applied_patches
}
_ => {
log::warn!("Unhandled VFS event: {:?}", event);
Expand Down Expand Up @@ -236,45 +262,6 @@ impl JobThreadContext {
}
}

// Find the nearest ancestor to this path that has
// associated instances in the tree. This helps make sure
// that we handle additions correctly, especially if we
// receive events for descendants of a large tree being
// created all at once.
fn on_vfs_event(
path: PathBuf,
tree: &Arc<Mutex<RojoTree>>,
vfs: &Arc<Vfs>,
) -> Vec<AppliedPatchSet> {
let mut tree = tree.lock().unwrap();
let mut applied_patches = Vec::new();

let mut current_path = path.as_path();
let affected_ids = loop {
let ids = tree.get_ids_at_path(current_path);

log::trace!("Path {} affects IDs {:?}", current_path.display(), ids);

if !ids.is_empty() {
break ids.to_vec();
}

log::trace!("Trying parent path...");
match current_path.parent() {
Some(parent) => current_path = parent,
None => break Vec::new(),
}
};

for id in affected_ids {
if let Some(patch) = compute_and_apply_changes(&mut tree, vfs, id) {
if !patch.is_empty() {
applied_patches.push(patch);
}
}
}
applied_patches
}
fn compute_and_apply_changes(tree: &mut RojoTree, vfs: &Vfs, id: Ref) -> Option<AppliedPatchSet> {
let metadata = tree
.get_metadata(id)
Expand Down
1 change: 0 additions & 1 deletion tests/tests/serve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ fn edit_init() {
});
}

#[ignore = "Does not pass on Github's runner"]
#[test]
fn move_folder_of_stuff() {
run_serve_test("move_folder_of_stuff", |session, mut redactions| {
Expand Down

0 comments on commit 097d39e

Please sign in to comment.