Fix a flaky test and work around issue 16003 in pkg/watcher.

pkg/watcher/lib/src/directory_watcher/mac_os.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library watcher.directory_watcher.mac_os;
 
 import 'dart:async';
 import 'dart:io';
 
 import 'package:path/path.dart' as p;
 import 'package:stack_trace/stack_trace.dart';
 
 import '../constructable_file_system_event.dart';
 import '../path_set.dart';
 import '../utils.dart';
 import '../watch_event.dart';
 import 'resubscribable.dart';
 
 /// Uses the FSEvents subsystem to watch for filesystem events.
 ///
 /// FSEvents has two main idiosyncrasies that this class works around. First, it
 /// will occasionally report events that occurred before the filesystem watch
 /// was initiated. Second, if multiple events happen to the same file in close
 /// succession, it won't report them in the order they occurred. See issue
 /// 14373.
 ///
-/// This also works around issue 14849 in the implementation of
+/// This also works around issues 16003 and 14849 in the implementation of
 /// [Directory.watch].
 class MacOSDirectoryWatcher extends ResubscribableDirectoryWatcher {
   // TODO(nweiz): remove these when issue 15042 is fixed.
   static var logDebugInfo = false;
   static var _count = 0;
 
   MacOSDirectoryWatcher(String directory)
       : super(directory, () => new _MacOSDirectoryWatcher(directory, _count++));
 }
 
@@ skipping 11 matching lines @@
 
   Future get ready => _readyCompleter.future;
   final _readyCompleter = new Completer();
 
   /// The number of event batches that have been received from
   /// [Directory.watch].
   ///
   /// This is used to determine if the [Directory.watch] stream was falsely
   /// closed due to issue 14849. A close caused by events in the past will only
   /// happen before or immediately after the first batch of events.
-  int batches = 0;
+  int _batches = 0;
 
   /// The set of files that are known to exist recursively within the watched
   /// directory.
   ///
   /// The state of files on the filesystem is compared against this to determine
   /// the real change that occurred when working around issue 14373. This is
   /// also used to emit REMOVE events when subdirectories are moved out of the
   /// watched directory.
   final PathSet _files;
 
@@ skipping 52 matching lines @@
       for (var event in batch) {
         print("[$_id]   ${_formatEvent(event)}");
       }
 
       print("[$_id] known files:");
       for (var file in _files.toSet()) {
         print("[$_id]   ${p.relative(file, from: directory)}");
       }
     }
 
-    batches++;
+    _batches++;
 
     _sortEvents(batch).forEach((path, events) {
       var relativePath = p.relative(path, from: directory);
       if (MacOSDirectoryWatcher.logDebugInfo) {
-        print("[$_id] events for $relativePath:\n");
+        print("[$_id] events for $relativePath:");
         for (var event in events) {
           print("[$_id]   ${_formatEvent(event)}");
         }
       }
 
       var canonicalEvent = _canonicalEvent(events);
       events = canonicalEvent == null ?
           _eventsBasedOnFileSystem(path) : [canonicalEvent];
       if (MacOSDirectoryWatcher.logDebugInfo) {
         print("[$_id] canonical event for $relativePath: "
@@ skipping 13 matching lines @@
             _emitEvent(ChangeType.ADD, path);
             _files.add(path);
             continue;
           }
 
           if (_files.containsDir(path)) continue;
 
           _listen(Chain.track(new Directory(path).list(recursive: true)),
               (entity) {
             if (entity is Directory) return;
+            if (_files.contains(path)) return;
+
             _emitEvent(ChangeType.ADD, entity.path);
             _files.add(entity.path);
           }, onError: (e, stackTrace) {
             if (MacOSDirectoryWatcher.logDebugInfo) {
               print("[$_id] got error listing $relativePath: $e");
             }
             _emitError(e, stackTrace);
           }, cancelOnError: true);
         } else if (event is FileSystemModifyEvent) {
           assert(!event.isDirectory);
@@ skipping 105 matching lines @@
     // from FSEvents reporting an add that happened prior to the watch
     // beginning. If we also received a MODIFY event, we want to report that,
     // but not the CREATE.
     if (type == FileSystemEvent.CREATE && hadModifyEvent &&
         _files.contains(batch.first.path)) {
       type = FileSystemEvent.MODIFY;
     }
 
     switch (type) {
       case FileSystemEvent.CREATE:
-        return new ConstructableFileSystemCreateEvent(batch.first.path, isDir);
+        // Issue 16003 means that a CREATE event for a directory can indicate
+        // that the directory was moved and then re-created.
+        // [_eventsBasedOnFileSystem] will handle this correctly by producing a
+        // DELETE event followed by a CREATE event if the directory exists.
+        if (isDir) return null;
+        return new ConstructableFileSystemCreateEvent(batch.first.path, false);
       case FileSystemEvent.DELETE:
-        return new ConstructableFileSystemCreateEvent(batch.first.path, false);
+        return new ConstructableFileSystemDeleteEvent(batch.first.path, isDir);
       case FileSystemEvent.MODIFY:
         return new ConstructableFileSystemModifyEvent(
             batch.first.path, isDir, false);
       default: assert(false);
     }
   }
 
   /// Returns one or more events that describe the change between the last known
   /// state of [path] and its current state on the filesystem.
   ///
   /// This returns a list whose order should be reflected in the events emitted
   /// to the user, unlike the batched events from [Directory.watch]. The
   /// returned list may be empty, indicating that no changes occurred to [path]
   /// (probably indicating that it was created and then immediately deleted).
   List<FileSystemEvent> _eventsBasedOnFileSystem(String path) {
     var fileExisted = _files.contains(path);
     var dirExisted = _files.containsDir(path);
     var fileExists = new File(path).existsSync();
     var dirExists = new Directory(path).existsSync();
 
     if (MacOSDirectoryWatcher.logDebugInfo) {
-      print("[$_id] file existed: $fileExisted");
-      print("[$_id] dir existed: $dirExisted");
-      print("[$_id] file exists: $fileExists");
-      print("[$_id] dir exists: $dirExists");
+      print("[$_id] checking file system for "
+          "${p.relative(path, from: directory)}");
+      print("[$_id]   file existed: $fileExisted");
+      print("[$_id]   dir existed: $dirExisted");
+      print("[$_id]   file exists: $fileExists");
+      print("[$_id]   dir exists: $dirExists");
     }
 
     var events = [];
     if (fileExisted) {
       if (fileExists) {
         events.add(new ConstructableFileSystemModifyEvent(path, false, false));
       } else {
         events.add(new ConstructableFileSystemDeleteEvent(path, false));
       }
     } else if (dirExisted) {
@@ skipping 12 matching lines @@
       events.add(new ConstructableFileSystemCreateEvent(path, false));
     } else if (!dirExisted && dirExists) {
       events.add(new ConstructableFileSystemCreateEvent(path, true));
     }
 
     return events;
   }
 
   /// The callback that's run when the [Directory.watch] stream is closed.
   void _onDone() {
+    print("[$_id] stream closed");

[Bob Nystrom, 2014/01/14 23:45:09]

Put this in if (MacOSDirectoryWatcher.logDebugInfo) and below.

[nweiz, 2014/01/14 23:49:07]

Good catch! Done.

+
     _watchSubscription = null;
 
     // If the directory still exists and we haven't seen more than one batch,
     // this is probably issue 14849 rather than a real close event. We should
     // just restart the watcher.
-    if (batches < 2 && new Directory(directory).existsSync()) {
+    if (_batches < 2 && new Directory(directory).existsSync()) {
+      print("[$_id] fake closure (issue 14849), re-opening stream");
       _startWatch();
       return;
     }
 
     // FSEvents can fail to report the contents of the directory being removed
-    // when the directory itself is removed, so we need to manually mark the as
-    // removed.
+    // when the directory itself is removed, so we need to manually mark the
+    // files as removed.
     for (var file in _files.toSet()) {
       _emitEvent(ChangeType.REMOVE, file);
     }
     _files.clear();
     close();
   }
 
   /// Start or restart the underlying [Directory.watch] stream.
   void _startWatch() {
     // Batch the FSEvent changes together so that we can dedup events.
@@ skipping 46 matching lines @@
     } else if (event is FileSystemDeleteEvent) {
       return "delete $type $path";
     } else if (event is FileSystemModifyEvent) {
       return "modify $type $path";
     } else if (event is FileSystemMoveEvent) {
       return "move $type $path to "
           "${p.relative(event.destination, from: directory)}";
     }
   }
 }