Merge #837

837: Generalize vfs.rs test to address fsevents timing-dependent behavior. r=matklad a=pnkfelix

Generalize `tests/vfs.rs` processing to address wildly-varying timing-dependent behavior from fsevents (i.e. on Mac OS X).

Fix #734 

Co-authored-by: Felix S. Klock II <[email protected]>

crates/ra_vfs/tests/vfs.rs

@@ -5,12 +5,27 @@ use crossbeam_channel::RecvTimeoutError;
 use ra_vfs::{Vfs, VfsChange};
 use tempfile::tempdir;
 
+/// Processes exactly `num_tasks` events waiting in the `vfs` message queue.
+///
+/// Panics if there are not exactly that many tasks enqueued for processing.
 fn process_tasks(vfs: &mut Vfs, num_tasks: u32) {
-    for _ in 0..num_tasks {
-        let task = vfs.task_receiver().recv_timeout(Duration::from_secs(3)).unwrap();
+    process_tasks_in_range(vfs, num_tasks, num_tasks);
+}
+
+/// Processes up to `max_count` events waiting in the `vfs` message queue.
+///
+/// Panics if it cannot process at least `min_count` events.
+/// Panics if more than `max_count` events are enqueued for processing.
+fn process_tasks_in_range(vfs: &mut Vfs, min_count: u32, max_count: u32) {
+    for i in 0..max_count {
+        let task = match vfs.task_receiver().recv_timeout(Duration::from_secs(3)) {
+            Err(RecvTimeoutError::Timeout) if i >= min_count => return,
+            otherwise => otherwise.unwrap(),
+        };
         log::debug!("{:?}", task);
         vfs.handle_task(task);
     }
+    assert!(vfs.task_receiver().is_empty());
 }
 
 macro_rules! assert_match {
@@ -67,6 +82,28 @@ fn test_vfs_works() -> std::io::Result<()> {
         assert_eq!(files, expected_files);
     }
 
+    // rust-analyzer#734: fsevents has a bunch of events still sitting around.
+    process_tasks_in_range(&mut vfs, 0, 7);
+    match vfs.commit_changes().as_slice() {
+        [] => {}
+
+        // This arises on fsevents (unless we wait 30 seconds before
+        // calling `Vfs::new` above). We need to churn through these
+        // events so that we can focus on the event that arises from
+        // the `fs::write` below.
+        [VfsChange::ChangeFile { .. }, // hello
+         VfsChange::ChangeFile { .. }, // world
+         VfsChange::AddFile { .. }, // b/baz.rs, nested hello
+         VfsChange::ChangeFile { .. }, // hello
+         VfsChange::ChangeFile { .. }, // world
+         VfsChange::ChangeFile { .. }, // nested hello
+         VfsChange::ChangeFile { .. }, // nested hello
+        ] => {}
+
+        changes => panic!("Expected events for setting up initial files, got: {GOT:?}",
+                          GOT=changes),
+    }
+
     fs::write(&dir.path().join("a/b/baz.rs"), "quux").unwrap();
     process_tasks(&mut vfs, 1);
     assert_match!(
@@ -118,18 +155,39 @@ fn test_vfs_works() -> std::io::Result<()> {
 
     fs::rename(&dir.path().join("a/sub1/sub2/new.rs"), &dir.path().join("a/sub1/sub2/new1.rs"))
         .unwrap();
-    // NOTE: Windows generates extra `Write` events when renaming?
-    // meaning we have extra tasks to process
-    process_tasks(&mut vfs, if cfg!(windows) { 4 } else { 2 });
-    assert_match!(
-        vfs.commit_changes().as_slice(),
-        [VfsChange::RemoveFile { path: removed_path, .. }, VfsChange::AddFile { text, path: added_path, .. }],
+
+    // rust-analyzer#734: For testing purposes, work-around
+    // passcod/notify#181 by processing either 1 or 2 events. (In
+    // particular, Mac can hand back either 1 or 2 events in a
+    // timing-dependent fashion.)
+    //
+    // rust-analyzer#827: Windows generates extra `Write` events when
+    // renaming? meaning we have extra tasks to process.
+    process_tasks_in_range(&mut vfs, 1, if cfg!(windows) { 4 } else { 2 });
+    match vfs.commit_changes().as_slice() {
+        [VfsChange::RemoveFile { path: removed_path, .. }, VfsChange::AddFile { text, path: added_path, .. }] =>
         {
             assert_eq!(removed_path, "sub1/sub2/new.rs");
             assert_eq!(added_path, "sub1/sub2/new1.rs");
             assert_eq!(text.as_str(), "new hello");
         }
-    );
+
+        // Hopefully passcod/notify#181 will be addressed in some
+        // manner that will reliably emit an event mentioning
+        // `sub1/sub2/new.rs`. But until then, must accept that
+        // debouncing loses information unrecoverably.
+        [VfsChange::AddFile { text, path: added_path, .. }] => {
+            assert_eq!(added_path, "sub1/sub2/new1.rs");
+            assert_eq!(text.as_str(), "new hello");
+        }
+
+        changes => panic!(
+            "Expected events for rename of {OLD} to {NEW}, got: {GOT:?}",
+            OLD = "sub1/sub2/new.rs",
+            NEW = "sub1/sub2/new1.rs",
+            GOT = changes
+        ),
+    }
 
     fs::remove_file(&dir.path().join("a/sub1/sub2/new1.rs")).unwrap();
     process_tasks(&mut vfs, 1);