From 097d39e8ce74c42697296d772e0458b0e45fd0d4 Mon Sep 17 00:00:00 2001
From: Kenneth Loeffler <kenloef@gmail.com>
Date: Sun, 31 Dec 2023 12:02:54 -0800
Subject: [PATCH] Fix move_folders_of_stuff (#826)

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
https://github.com/rojo-rbx/rojo/commit/b43b45be8f2eb01161a389afeab56749fa82e459.
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.
---
 src/change_processor.rs | 81 +++++++++++++++++------------------------
 tests/tests/serve.rs    |  1 -
 2 files changed, 34 insertions(+), 48 deletions(-)

diff --git a/src/change_processor.rs b/src/change_processor.rs
index 2429b5903..497ba4e79 100644
--- a/src/change_processor.rs
+++ b/src/change_processor.rs
@@ -1,6 +1,5 @@
 use std::{
     fs,
-    path::PathBuf,
     sync::{Arc, Mutex},
 };
 
@@ -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);
@@ -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)
diff --git a/tests/tests/serve.rs b/tests/tests/serve.rs
index 9058c3662..d55d339bb 100644
--- a/tests/tests/serve.rs
+++ b/tests/tests/serve.rs
@@ -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| {