File watching package.

pkg/watcher/test/utils.dart

Index: pkg/watcher/test/utils.dart
diff --git a/pkg/watcher/test/utils.dart b/pkg/watcher/test/utils.dart
new file mode 100644
index 0000000000000000000000000000000000000000..80603a4ad6d8384d95572be8b68289189de32085
--- /dev/null
+++ b/pkg/watcher/test/utils.dart
@@ -0,0 +1,196 @@
+// Copyright (c) 2012, 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.test.utils;
+
+import 'dart:async';
+import 'dart:io';
+
+import 'package:pathos/path.dart' as pathos;
+import 'package:scheduled_test/scheduled_test.dart';
+import 'package:watcher/watcher.dart';
+import 'package:watcher/src/stat.dart';
+
+// TODO(rnystrom): Find a better way to use this.

[Siggi Cherem (dart-lang), 2013/07/10 22:53:53]

move the config to unittest?

For now, make sure this package is excluded from the script that publish
packages from trunk to pub.

[Bob Nystrom, 2013/07/10 23:04:39]

It depends on stack_trace and, I think, some other stuff. Is it OK for unittest
to depend on those?
I know it's nasty, but I'm personally OK with letting this be published. You can
still use the package, you just can't run its tests. I can remove this line if
you'd prefer.

[Siggi Cherem (dart-lang), 2013/07/10 23:25:31]

Eventually yes, I believe Nathan was already working on using stack_strace in
unittest, but he had to roll it back temporarily. I can't recall why.
I'm ok either way. I slightly prefer removing it to make it self contained.

[nweiz, 2013/07/11 00:25:01]

It caused a VM crash. asiva@ is working on it.

[Bob Nystrom, 2013/07/11 18:33:20]

Yup, I'll remove this and switch it out to use compact config once that lands.

+import '../../../sdk/lib/_internal/pub/test/command_line_config.dart';
+
+var _configured = false;
+
+String _sandboxDir;
+DirectoryWatcher _watcher;
+var _nextEvent = 0;

[nweiz, 2013/07/11 00:25:01]

It would be nice to document all of these variables, but [_nextEvent] in
particular is unclear without further context.

[Bob Nystrom, 2013/07/11 18:33:20]

Done.

+
+/// The mock modification times (in milliseconds since epoch) for each file.
+///
+/// The actual file system has pretty high granularity for file modification

[nweiz, 2013/07/11 00:25:01]

Do you mean "low granularity"?

[Bob Nystrom, 2013/07/11 18:33:20]

Damn ambiguity of directional metaphors! Changed to "coarse".

+/// times. This means using the real file system requires us to put delays in
+/// the tests to ensure we wait long enough between operations for the mod time
+/// to be different.
+///
+/// Instead, we'll just mock that out. Each time a file is written, we manually
+/// increment the mod time for that file instantly.
+Map<String, int> _mockFileModificationTimes;
+
+void initConfig() {
+  if (_configured) return;
+  _configured = true;
+  unittestConfiguration = new CommandLineConfiguration();
+}
+
+/// Creates a new [DirectoryWatcher] that watches a temporary directory.
+///
+/// When the current schedule completes, the directory is deleted.
+DirectoryWatcher createWatcher() {
+  _ensureSandbox();

[nweiz, 2013/07/11 00:25:01]

Is calling this everywhere really cleaner than creating a wrapper for [test]
like we do in pub, or using [setUp]?

[Bob Nystrom, 2013/07/11 18:33:20]

I kind of like it. Wrapping test() is a bit tedious because you have to hide the
other import and then handle both test() and solo_test().

I could make initConfig() call setUp(), but that feels a bit magical, and does
unnecessary work if there are later unit tests that don't need the sandbox. I
like that this only creates a sandbox for tests that use it.

[nweiz, 2013/07/11 22:29:00]

You should just call [setUp] explicitly. You only have one test file; there's no
need for unnecessary abstraction to support multiple files that don't exist yet.
Every test uses it.

[Bob Nystrom, 2013/07/12 00:31:09]

I'm certain more tests will exist and the current code works fine and isn't bad.
Why is it worth changing it?

[nweiz, 2013/07/12 01:04:28]

Why will more tests exist? Are you planning on adding additional functionality?
If those tests require the same setup, why can't they use the same setUp
callback? If they don't, why not put them in a different file?
Because this way of writing it is worse than the alternative. It adds additional
global state, it requires that you remember to put "_ensureSandbox" at the
beginning of every utility function, and it's a custom-rolled way of doing
something where the standard solution (setUp) would work just fine.

[Bob Nystrom, 2013/07/12 17:46:16]

Probably. At the very least, I'm thinking of expanding this to have watchers for
watching a collection of directories, or a set of fixed file paths.
They will be in a different file, but they'll still use utils.dart and
initConfig().
OK, done. My original issue was that initConfig() isn't the right place for
this. So I added a separate createSandbox() function and made the test suite do
setUp(createSandbox). That way the setUp() call is still in the test where it's
clearly visible and other tests can call initConfig() without getting a sandbox
they don't care about.

+  _watcher = new DirectoryWatcher(_sandboxDir);
+
+  // When a listener is first registered on the watcher, it scans the directory
+  // to see the set of pre-existing files. This way, it doesn't report
+  // notifications for files that were there before the watcher started.
+  //
+  // The scan is done asynchronously so that creating a watcher is fast, and
+  // we don't have an exposed API to know when that initial scan is done.
+  // Since many tests need to make changes after that scan, we need to ensure
+  // that we don't start modifying things until its had time to complete.
+  //
+  // This waits some tuned amount of time to ensure that's happened.
+  schedule(() => new Future.delayed(new Duration(milliseconds: 50)));

[nweiz, 2013/07/11 00:25:01]

Explicit timeouts are very dangerous when dealing with the bots, since they're
so much slower than our computers. I'm confident 50ms won't be enough here.

Another possibility here is to use a sentinel file. Keep modifying a single file
in the sandbox directory until that modification is reported; once it is, we
know the watcher is initialized.

[Bob Nystrom, 2013/07/11 18:33:20]

I know. My plan was to tune it upwards until the bots were reliably stable. Yes,
it's gross. :(
I started hacking on that, but the code is pretty long and nasty. It also
interferes with the behavior under test: if it's waiting for events then you
can't write tests for things like the first event, or what happens before events
are sent.

Another possible option would be have a private API (like src/stat.dart) that
exposes a stream that emits an object when the initial scan is done. This could
then wait for that.

That would be reliable and simple, but I'm not crazy about adding code to the
library that exists just for testing. Thoughts?

[nweiz, 2013/07/11 22:29:00]

Given the state of the bots, I think pretty much anything is better than using a
timeout to wait for processing to be done. It won't just involve tweaking this
CL multiple times on submit; it'll involve re-tweaking it in the future each
time the bots get slower.

It actually may be a good idea to expose an onReady event anyway. For example,
in barback, we probably only want to run the initial compile once the first scan
has been completed. If we start compiling before that, we run the risk of a file
changing after we read it but before the initial scan reaches it.

[Bob Nystrom, 2013/07/12 00:31:09]

Done.

Calling it "ready" is nice and ambiguous and doesn't couple it to polling.

+
+  currentSchedule.onComplete.schedule(() {
+    _nextEvent = 0;
+    _watcher = null;
+  }, "reset watcher");
+
+  return _watcher;
+}
+
+void expectEvent(ChangeType type, String path) {
+  // Immediately create the future. This ensures we don't register too late and
+  // the event gets dropped before we receive it.

[nweiz, 2013/07/11 00:25:01]

"the event gets dropped" -> "drop the event"

[Bob Nystrom, 2013/07/11 18:33:20]

Done.

+  var future = expect(_watcher.events.elementAt(_nextEvent++).then((event) {
+    expect(event, new _ChangeMatcher(type, path));
+  }), completes);

[nweiz, 2013/07/11 00:25:01]

expect() doesn't return a Future. I think you want to use
currentSchedule.wrapFuture here instead.

[Bob Nystrom, 2013/07/11 18:33:20]

Done.

+
+  // Then schedule the expectation on the future. This ensures that later
+  // scheduled stuff doesn't happen until after this event is received.
+  schedule(() => future, "wait for $type $path event");
+}
+
+void expectAddEvent(String path) {
+  _ensureSandbox();

[nweiz, 2013/07/11 00:25:01]

This seems redundant, since [createWatcher] must be called before this anyway.

[Bob Nystrom, 2013/07/11 18:33:20]

Removed.

+  expectEvent(ChangeType.ADD, pathos.join(_sandboxDir, path));
+}
+
+void expectModifyEvent(String path) {
+  _ensureSandbox();
+  expectEvent(ChangeType.MODIFY, pathos.join(_sandboxDir, path));
+}
+
+void expectRemoveEvent(String path) {
+  _ensureSandbox();
+  expectEvent(ChangeType.REMOVE, pathos.join(_sandboxDir, path));
+}
+
+/// Writes a file in the sandbox at [path] with [contents].
+///
+/// If [contents] is omitted, creates an empty file. If [updatedModified] is
+/// `false`, the mock file modification time is not changed.
+void writeFile(String path, {String contents, bool updateModified}) {

[nweiz, 2013/07/11 00:25:01]

This should be scheduled, as should removeFile and renameFile.

[Bob Nystrom, 2013/07/11 18:33:20]

Done.

+  if (contents == null) contents = "";
+  if (updateModified == null) updateModified = true;
+
+  _ensureSandbox();
+
+  var fullPath = pathos.join(_sandboxDir, path);
+
+  // Create any needed subdirectories.
+  var dir = new Directory(pathos.dirname(fullPath));
+  if (!dir.existsSync()) {
+    dir.createSync(recursive: true);
+  }
+
+  new File(fullPath).writeAsStringSync(contents);
+
+  // Manually update the mock modification time for the file.
+  if (updateModified) {
+    var milliseconds = _mockFileModificationTimes.putIfAbsent(path, () => 0);
+    _mockFileModificationTimes[path]++;
+  }
+}
+
+/// Deletes a file in the sandbox at [path].
+void deleteFile(String path) {
+  _ensureSandbox();
+  new File(pathos.join(_sandboxDir, path)).deleteSync();
+}
+
+/// Renames a file in the sandbox from [from] to [to].
+///
+/// If [contents] is omitted, creates an empty file.
+void renameFile(String from, String to) {
+  _ensureSandbox();
+
+  new File(pathos.join(_sandboxDir, from)).renameSync(
+      pathos.join(_sandboxDir, to));
+
+  // Manually update the mock modification time for the file.
+  var milliseconds = _mockFileModificationTimes.putIfAbsent(to, () => 0);
+  _mockFileModificationTimes[to]++;
+}
+
+/// Wait an appropriate amount of time to ensure that the watcher has had time
+/// to walk the entire directory contents.
+///
+/// The watcher repeately lists the contents of the directory and notifies
+/// based on the differences it finds between successive runs. That means for
+/// some tests, we need to ensure that we don't start modifying things until
+/// a listing has had time to complete.
+///
+/// This waits some tuned amount of time to ensure that's happened.
+waitForListing() {
+  schedule(() => new Future.delayed(new Duration(milliseconds: 500)));
+}

[nweiz, 2013/07/11 00:25:01]

This function isn't called anywhere.

[Bob Nystrom, 2013/07/11 18:33:20]

Oops. Old code. Removed.

+
+/// Makes sure the sandbox directory has been created for this schedule.
+void _ensureSandbox() {
+  if (_sandboxDir == null) {

[nweiz, 2013/07/11 00:25:01]

Feels like this should short-circuit.

[Bob Nystrom, 2013/07/11 18:33:20]

Done.

+    var dir = new Directory("").createTempSync();
+    _sandboxDir = dir.path;
+
+    _mockFileModificationTimes = new Map<String, int>();
+    mockGetModificationTime((path) {
+      path = pathos.relative(path, from: _sandboxDir);

[nweiz, 2013/07/11 00:25:01]

Add an assert that this works.

[Bob Nystrom, 2013/07/11 18:33:20]

Done.

+      return new DateTime.fromMillisecondsSinceEpoch(
+          _mockFileModificationTimes[path]);
+    });
+
+    currentSchedule.onComplete.schedule(() {
+      if (_sandboxDir != null) {
+        new Directory(_sandboxDir).deleteSync(recursive: true);
+        _sandboxDir = null;
+      }
+
+      _mockFileModificationTimes = null;
+      mockGetModificationTime(null);
+    }, "delete sandbox");
+  }
+}
+
+/// A [Matcher] for [WatchEvent]s.
+class _ChangeMatcher extends BaseMatcher {
+  /// The expected change.
+  final ChangeType type;
+
+  /// The expected path.
+  final String path;
+
+  _ChangeMatcher(this.type, this.path);
+
+  Description describe(Description description) {
+    description.add("$type $path");
+  }
+
+  bool matches(item, Map matchState) =>
+      item is WatchEvent && item.type == type && item.path == path;
+}