Fix PollingFileWatcher.ready for files that don't exist (dart-archive/watcher#157)

There were a few issues here:

- FileWatcher.ready never fired for files that don't exist because of logic inside FileWatcher existing early if the modification time was `null`
- The test I recently added trying to catch this was incorrectly passing because the mock timestamp code was set so that files that had not been created would return a 0-mtime whereas in the real implementation they return `null`

So this change

a) updates the mock to return `null` for uncreated files (to match the real implementation) which breaks a bunch of tests

b) fixes those tests by updating FileWatcher._poll() to handle `null` mtime separately from being the first poll.

Author: DanTup

pkgs/watcher/CHANGELOG.md

@@ -1,5 +1,7 @@
 ## 1.1.1-wip
 
+- Ensure `PollingFileWatcher.ready` completes for files that do not exist.
+
 ## 1.1.0
 
 - Require Dart SDK >= 3.0.0

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

@@ -39,8 +39,7 @@ class _PollingFileWatcher implements FileWatcher, ManuallyClosedWatcher {
 
   /// The previous modification time of the file.
   ///
-  /// Used to tell when the file was modified. This is `null` before the file's
-  /// mtime has first been checked.
+  /// `null` indicates the file does not (or did not on the last poll) exist.
   DateTime? _lastModified;
 
   _PollingFileWatcher(this.path, Duration pollingDelay) {
@@ -50,13 +49,14 @@ class _PollingFileWatcher implements FileWatcher, ManuallyClosedWatcher {
 
   /// Checks the mtime of the file and whether it's been removed.
   Future<void> _poll() async {
-    // We don't mark the file as removed if this is the first poll (indicated by
-    // [_lastModified] being null). Instead, below we forward the dart:io error
-    // that comes from trying to read the mtime below.
+    // We don't mark the file as removed if this is the first poll. Instead,
+    // below we forward the dart:io error that comes from trying to read the
+    // mtime below.
     var pathExists = await File(path).exists();
     if (_eventsController.isClosed) return;
 
     if (_lastModified != null && !pathExists) {
+      _flagReady();
       _eventsController.add(WatchEvent(ChangeType.REMOVE, path));
       unawaited(close());
       return;
@@ -67,22 +67,34 @@ class _PollingFileWatcher implements FileWatcher, ManuallyClosedWatcher {
       modified = await modificationTime(path);
     } on FileSystemException catch (error, stackTrace) {
       if (!_eventsController.isClosed) {
+        _flagReady();
         _eventsController.addError(error, stackTrace);
         await close();
       }
     }
-    if (_eventsController.isClosed) return;
-
-    if (_lastModified == modified) return;
+    if (_eventsController.isClosed) {
+      _flagReady();
+      return;
+    }
 
-    if (_lastModified == null) {
+    if (!isReady) {
       // If this is the first poll, don't emit an event, just set the last mtime
       // and complete the completer.
       _lastModified = modified;
+      _flagReady();
+      return;
+    }
+
+    if (_lastModified == modified) return;
+
+    _lastModified = modified;
+    _eventsController.add(WatchEvent(ChangeType.MODIFY, path));
+  }
+
+  /// Flags this watcher as ready if it has not already been done.
+  void _flagReady() {
+    if (!isReady) {
       _readyCompleter.complete();
-    } else {
-      _lastModified = modified;
-      _eventsController.add(WatchEvent(ChangeType.MODIFY, path));
     }
   }
 

pkgs/watcher/lib/src/stat.dart

@@ -6,7 +6,7 @@ import 'dart:io';
 
 /// A function that takes a file path and returns the last modified time for
 /// the file at that path.
-typedef MockTimeCallback = DateTime Function(String path);
+typedef MockTimeCallback = DateTime? Function(String path);
 
 MockTimeCallback? _mockTimeCallback;
 

pkgs/watcher/test/utils.dart

@@ -67,7 +67,7 @@ Future<void> startWatcher({String? path}) async {
         'Path is not in the sandbox: $path not in ${d.sandbox}');
 
     var mtime = _mockFileModificationTimes[normalized];
-    return DateTime.fromMillisecondsSinceEpoch(mtime ?? 0);
+    return mtime != null ? DateTime.fromMillisecondsSinceEpoch(mtime) : null;
   });
 
   // We want to wait until we're ready *after* we subscribe to the watcher's
@@ -195,6 +195,11 @@ Future expectRemoveEvent(String path) =>
 Future allowModifyEvent(String path) =>
     _expectOrCollect(mayEmit(isWatchEvent(ChangeType.MODIFY, path)));
 
+/// Track a fake timestamp to be used when writing files. This always increases
+/// so that files that are deleted and re-created do not have their timestamp
+/// set back to a previously used value.
+int _nextTimestamp = 1;
+
 /// Schedules writing a file in the sandbox at [path] with [contents].
 ///
 /// If [contents] is omitted, creates an empty file. If [updateModified] is
@@ -216,14 +221,15 @@ void writeFile(String path, {String? contents, bool? updateModified}) {
   if (updateModified) {
     path = p.normalize(path);
 
-    _mockFileModificationTimes.update(path, (value) => value + 1,
-        ifAbsent: () => 1);
+    _mockFileModificationTimes[path] = _nextTimestamp++;
   }
 }
 
 /// Schedules deleting a file in the sandbox at [path].
 void deleteFile(String path) {
   File(p.join(d.sandbox, path)).deleteSync();
+
+  _mockFileModificationTimes.remove(path);
 }
 
 /// Schedules renaming a file in the sandbox from [from] to [to].
@@ -245,6 +251,16 @@ void createDir(String path) {
 /// Schedules renaming a directory in the sandbox from [from] to [to].
 void renameDir(String from, String to) {
   Directory(p.join(d.sandbox, from)).renameSync(p.join(d.sandbox, to));
+
+  // Migrate timestamps for any files in this folder.
+  final knownFilePaths = _mockFileModificationTimes.keys.toList();
+  for (final filePath in knownFilePaths) {
+    if (p.isWithin(from, filePath)) {
+      _mockFileModificationTimes[filePath.replaceAll(from, to)] =
+          _mockFileModificationTimes[filePath]!;
+      _mockFileModificationTimes.remove(filePath);
+    }
+  }
 }
 
 /// Schedules deleting a directory in the sandbox at [path].