Fix for skipped tests on Windows. (#2204)

Author: davidmorgan

pkgs/watcher/CHANGELOG.md

@@ -4,6 +4,11 @@
   the file was created immediately before the watcher was created. Now, if the
   file exists when the watcher is created then this modify event is not sent.
   This matches the Linux native and polling (Windows) watchers.
+- Bug fix: with `DirectoryWatcher` on Windows, a move over an existing file was
+  reported incorrectly. For example, if `a` and `b` already exist, then `a` is
+  moved onto `b`, it would be reported as three events: delete `a`, delete `b`,
+  create `b`. Now it's reported as two events: delete `a`, modify `b`. This
+  matches the behavior of the Linux and MacOS watchers.
 
 ## 1.1.4
 

pkgs/watcher/lib/src/directory_watcher/windows.dart

@@ -30,10 +30,8 @@ class _EventBatcher {
   final List<Event> events = [];
   Timer? timer;
 
-  void addEvent(FileSystemEvent event, void Function() callback) {
-    final convertedEvent = Event.checkAndConvert(event);
-    if (convertedEvent == null) return;
-    events.add(convertedEvent);
+  void addEvent(Event event, void Function() callback) {
+    events.add(event);
     timer?.cancel();
     timer = Timer(_batchDelay, callback);
   }
@@ -166,8 +164,28 @@ class _WindowsDirectoryWatcher
     );
   }
 
-  void _onEvent(FileSystemEvent event) {
+  void _onEvent(FileSystemEvent fileSystemEvent) {
     assert(isReady);
+    final event = Event.checkAndConvert(fileSystemEvent);
+    if (event == null) return;
+    if (event.type == EventType.moveFile) {
+      _batchEvent(Event.delete(event.path));
+      final destination = event.destination;
+      if (destination != null) {
+        _batchEvent(Event.createFile(destination));
+      }
+    } else if (event.type == EventType.moveDirectory) {
+      _batchEvent(Event.delete(event.path));
+      final destination = event.destination;
+      if (destination != null) {
+        _batchEvent(Event.createDirectory(destination));
+      }
+    } else {
+      _batchEvent(event);
+    }
+  }
+
+  void _batchEvent(Event event) {
     final batcher = _eventBatchers.putIfAbsent(event.path, _EventBatcher.new);
     batcher.addEvent(event, () {
       _eventBatchers.remove(event.path);
@@ -225,7 +243,7 @@ class _WindowsDirectoryWatcher
               _emitEvent(ChangeType.REMOVE, removedPath);
             }
 
-          // Move events are removed by `_canonicalEvent` and never returned by
+          // Move events are removed by `_onEvent` and never returned by
           // `_eventsBasedOnFileSystem`.
           case EventType.moveFile:
           case EventType.moveDirectory:
@@ -240,38 +258,27 @@ class _WindowsDirectoryWatcher
   }
 
   /// Sort all the events in a batch into sets based on their path.
-  ///
-  /// Events for [path] are discarded.
   Map<String, Set<Event>> _sortEvents(List<Event> batch) {
     var eventsForPaths = <String, Set<Event>>{};
 
-    // Events within created or moved directories are not needed as the
-    // directory's full contents will be listed.
-    var directories = unionAll(
-      batch.map((event) {
-        if (event.type == EventType.createDirectory ||
-            event.type == EventType.moveDirectory) {
-          final destination = event.destination;
-          return {event.path, if (destination != null) destination};
-        }
-        return const <String>{};
-      }),
-    );
+    // Events within directories that already have create events are not needed
+    // as the directory's full content will be listed.
+    var createdDirectories = unionAll(batch.map((event) {
+      return event.type == EventType.createDirectory
+          ? {event.path}
+          : const <String>{};
+    }));
 
-    bool isInModifiedDirectory(String path) =>
-        directories.any((dir) => path != dir && p.isWithin(dir, path));
+    bool isInCreatedDirectory(String path) =>
+        createdDirectories.any((dir) => path != dir && p.isWithin(dir, path));
 
     void addEvent(String path, Event event) {
-      if (isInModifiedDirectory(path)) return;
+      if (isInCreatedDirectory(path)) return;
       eventsForPaths.putIfAbsent(path, () => <Event>{}).add(event);
     }
 
     for (var event in batch) {
       addEvent(event.path, event);
-      final destination = event.destination;
-      if (destination != null) {
-        addEvent(destination, event);
-      }
     }
 
     return eventsForPaths;
@@ -300,11 +307,6 @@ class _WindowsDirectoryWatcher
       return null;
     }
 
-    // Move events are always resolved by checking the filesystem.
-    if (type == EventType.moveFile || type == EventType.moveDirectory) {
-      return null;
-    }
-
     return batch.firstWhere((e) => e.type == type);
   }
 

pkgs/watcher/test/directory_watcher/shared.dart

@@ -137,8 +137,6 @@ void sharedTests() {
 
       renameFile('from.txt', 'to.txt');
       await inAnyOrder([isRemoveEvent('from.txt'), isModifyEvent('to.txt')]);
-    }, onPlatform: {
-      'windows': const Skip('https://github.com/dart-lang/watcher/issues/125')
     });
   });
 
@@ -280,8 +278,6 @@ void sharedTests() {
         isRemoveEvent('old'),
         isAddEvent('new')
       ]);
-    }, onPlatform: {
-      'windows': const Skip('https://github.com/dart-lang/watcher/issues/21')
     });
 
     test('emits events for many nested files added at once', () async {