Fix PollingDirectoryWatcher for delete during poll. (#2212)

Author: davidmorgan

pkgs/watcher/CHANGELOG.md

@@ -9,6 +9,8 @@
   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.
+- Bug fix: with `PollingDirectoryWatcher`, fix spurious modify event emitted
+  because of a file delete during polling.
 
 ## 1.1.4
 

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

@@ -160,8 +160,13 @@ class _PollingDirectoryWatcher
 
     if (_events.isClosed) return;
 
-    _lastModifieds[file] = modified;
     _polledFiles.add(file);
+    if (modified == null) {
+      // The file was in the directory listing but has been removed since then.
+      // Don't add to _lastModifieds, it will be reported as a REMOVE.
+      return;
+    }
+    _lastModifieds[file] = modified;
 
     // Only notify if we're ready to emit events.
     if (!isReady) return;

pkgs/watcher/test/directory_watcher/polling_test.dart

@@ -32,6 +32,39 @@ void main() {
       writeFile('b.txt', contents: 'after');
       await expectModifyEvent('b.txt');
     });
+
+    // A poll does an async directory list then checks mtime on each file. Check
+    // handling of a file that is deleted between the two.
+    test('deletes during poll', () async {
+      await startWatcher();
+
+      for (var i = 0; i != 300; ++i) {
+        writeFile('$i');
+      }
+      // A series of deletes with delays in between for 300ms, which will
+      // intersect with the 100ms polling multiple times.
+      for (var i = 0; i != 300; ++i) {
+        deleteFile('$i');
+        await Future<void>.delayed(const Duration(milliseconds: 1));
+      }
+
+      final events =
+          await takeEvents(duration: const Duration(milliseconds: 500));
+
+      // Events should be adds and removes that pair up, with no modify events.
+      final adds = <String>{};
+      final removes = <String>{};
+      for (var event in events) {
+        if (event.type == ChangeType.ADD) {
+          adds.add(event.path);
+        } else if (event.type == ChangeType.REMOVE) {
+          removes.add(event.path);
+        } else {
+          fail('Unexpected event: $event');
+        }
+      }
+      expect(adds, removes);
+    });
   });
 
   // Also test with delayed writes and real mtimes.

pkgs/watcher/test/utils.dart

@@ -254,11 +254,24 @@ Future<WatchEvent?> waitForEvent({
   return result;
 }
 
-/// Expects that no events are omitted for [duration].
+/// Expects that no events are emitted for [duration].
 Future expectNoEvents({Duration duration = const Duration(seconds: 1)}) async {
   expect(await waitForEvent(duration: duration), isNull);
 }
 
+/// Takes all events emitted for [duration].
+Future<List<WatchEvent>> takeEvents({required Duration duration}) async {
+  final result = <WatchEvent>[];
+  final stopwatch = Stopwatch()..start();
+  while (stopwatch.elapsed < duration) {
+    final event = await waitForEvent(duration: duration - stopwatch.elapsed);
+    if (event != null) {
+      result.add(event);
+    }
+  }
+  return result;
+}
+
 /// Expects that the next event emitted will be for an add event for [path].
 Future expectAddEvent(String path) =>
     _expectOrCollect(isWatchEvent(ChangeType.ADD, path));
@@ -445,7 +458,9 @@ void renameDir(String from, String to) {
     final knownFilePaths = mockFileModificationTimes.keys.toList();
     for (final filePath in knownFilePaths) {
       if (p.isWithin(from, filePath)) {
-        mockFileModificationTimes[filePath.replaceAll(from, to)] =
+        final movedPath =
+            p.normalize(p.join(to, filePath.substring(from.length + 1)));
+        mockFileModificationTimes[movedPath] =
             mockFileModificationTimes[filePath]!;
         mockFileModificationTimes.remove(filePath);
       }