Refactor the barback tests somewhat.

pkg/barback/test/utils.dart

Index: pkg/barback/test/utils.dart
diff --git a/pkg/barback/test/utils.dart b/pkg/barback/test/utils.dart
index fcda96266b3e226e1f6c97f9b8d42f64fb9f8072..43d3777fbb494963dbe06c30e2c7c56b8336d98a 100644
--- a/pkg/barback/test/utils.dart
+++ b/pkg/barback/test/utils.dart
@@ -10,6 +10,8 @@ import 'dart:io';
 
 import 'package:barback/barback.dart';
 import 'package:barback/src/asset_cascade.dart';
+import 'package:barback/src/asset_set.dart';
+import 'package:barback/src/cancelable_future.dart';
 import 'package:barback/src/package_graph.dart';
 import 'package:barback/src/utils.dart';
 import 'package:path/path.dart' as pathos;
@@ -17,6 +19,14 @@ import 'package:scheduled_test/scheduled_test.dart';
 import 'package:stack_trace/stack_trace.dart';
 import 'package:unittest/compact_vm_config.dart';
 
+export 'transformer/bad.dart';
+export 'transformer/check_content.dart';
+export 'transformer/create_asset.dart';
+export 'transformer/many_to_one.dart';
+export 'transformer/mock.dart';
+export 'transformer/one_to_many.dart';
+export 'transformer/rewrite.dart';
+
 var _configured = false;
 
 MockProvider _provider;
@@ -96,6 +106,15 @@ void modifyAsset(String name, String contents) {
   }, "modify asset $name");
 }
 
+/// Schedules an error when loading the asset identified by [name].

[Bob Nystrom, 2013/07/30 22:25:02]

"error" -> "error to be generated".

[nweiz, 2013/07/30 22:46:21]

Done.

+///
+/// Does not update the asset in the graph.
+void setAssetError(String name) {
+  schedule(() {
+    _provider._setAssetError(name);
+  }, "set error for asset $name");
+}
+
 /// Schedules a pause of the internally created [PackageProvider].
 ///
 /// All asset requests that the [PackageGraph] makes to the provider after this
@@ -117,31 +136,16 @@ void resumeProvider() {
 /// [buildShouldFail], so those can be used to validate build results before and
 /// after this.
 void buildShouldNotBeDone() {
-  var resultAllowed = false;
-  var trace = new Trace.current();
-  _graph.results.elementAt(_nextBuildResult).then((result) {
-    if (resultAllowed) return;
-
-    currentSchedule.signalError(
-        new Exception("Expected build not to terminate "
-            "here, but it terminated with result: $result"), trace);
-  }).catchError((error) {
-    if (resultAllowed) return;
-    currentSchedule.signalError(error);
-  });
-
-  schedule(() {
-    // Pump the event queue in case the build completes out-of-band after we get
-    // here. If it does, we want to signal an error.
-    return pumpEventQueue().then((_) {
-      resultAllowed = true;
-    });
-  }, "ensuring build doesn't terminate");
+  _futureShouldNotCompleteUntil(
+      _graph.results.elementAt(_nextBuildResult),
+      schedule(() => pumpEventQueue(), "build should not terminate"),
+      "build");
 }
 
 /// Expects that the next [BuildResult] is a build success.
 void buildShouldSucceed() {
   expect(_getNextBuildResult().then((result) {
+    result.errors.forEach(currentSchedule.signalError);
     expect(result.succeeded, isTrue);
   }), completes);
 }
@@ -190,9 +194,8 @@ void expectAsset(String name, [String contents]) {
   schedule(() {
     return _graph.getAssetById(id).then((asset) {
       // TODO(rnystrom): Make an actual Matcher class for this.
-      expect(asset, new isInstanceOf<MockAsset>());
       expect(asset.id, equals(id));
-      expect(asset.contents, equals(contents));
+      expect(asset.readAsString(), completion(equals(contents)));
     });
   }, "get asset $name");
 }
@@ -213,6 +216,19 @@ void expectNoAsset(String name) {
   }, "get asset $name");
 }
 
+/// Schedules an expectation that a [getAssetById] call for the given asset
+/// won't terminate at this point in the schedule.
+void expectAssetDoesNotComplete(String name) {
+  var id = new AssetId.parse(name);
+
+  schedule(() {
+    return _futureShouldNotCompleteUntil(
+        _graph.getAssetById(id),
+        pumpEventQueue(),
+        "asset $id");
+  }, "asset $id should not complete");
+}
+
 /// Returns a matcher for an [AssetNotFoundException] with the given [id].
 Matcher isAssetNotFoundException(String name) {
   var id = new AssetId.parse(name);
@@ -247,12 +263,44 @@ Matcher isInvalidOutputException(String package, String name) {
       predicate((error) => error.id == id, 'id is $name'));
 }
 
+/// Returns a matcher for a [MockLoadException] with the given [id].
+Matcher isMockLoadException(String name) {
+  var id = new AssetId.parse(name);
+  return allOf(
+      new isInstanceOf<MockLoadException>(),
+      predicate((error) => error.id == id, 'id is $name'));

[Bob Nystrom, 2013/07/30 22:25:02]

"is" -> "=="

[nweiz, 2013/07/30 22:46:21]

Done.

+}
+
+/// Asserts that [future] shouldn't complete until after [delay] completes.
+///
+/// Once [delay] completes, the output of [future] is ignored, even if it's an
+/// error.
+///
+/// [description] should describe [future].
+Future _futureShouldNotCompleteUntil(Future future, Future delay,
+    String description) {
+  var trace = new Trace.current();
+  var cancelable = new CancelableFuture(future);
+  cancelable.then((result) {
+    currentSchedule.signalError(
+        new Exception("Expected $description not to complete here, but it "
+            "completed with result: $result"),
+        trace);
+  }).catchError((error) {
+    currentSchedule.signalError(error);
+  });
+
+  return delay.then((_) => cancelable.cancel());
+}
+
 /// An [AssetProvider] that provides the given set of assets.
 class MockProvider implements PackageProvider {
   Iterable<String> get packages => _packages.keys;
 
   Map<String, _MockPackage> _packages;
 
+  final _errors = new Set<AssetId>();

[Bob Nystrom, 2013/07/30 22:25:02]

Document.

[nweiz, 2013/07/30 22:46:21]

Done.

+
   /// The completer that [getAsset()] is waiting on to complete when paused.
   ///
   /// If `null` it will return the asset immediately.
@@ -277,13 +325,13 @@ class MockProvider implements PackageProvider {
     if (assets is Map) {
       assetList = assets.keys.map((asset) {
         var id = new AssetId.parse(asset);
-        return new MockAsset(id, assets[asset]);
+        return new _MockAsset(id, assets[asset]);
       });
     } else if (assets is Iterable) {
       assetList = assets.map((asset) {
         var id = new AssetId.parse(asset);
         var contents = pathos.basenameWithoutExtension(id.path);
-        return new MockAsset(id, contents);
+        return new _MockAsset(id, contents);
       });
     }
 
@@ -291,21 +339,26 @@ class MockProvider implements PackageProvider {
         (package, assets) {
       var packageTransformers = transformers[package];
       if (packageTransformers == null) packageTransformers = [];
-      return new _MockPackage(assets, packageTransformers.toList());
+      return new _MockPackage(
+          new AssetSet.from(assets), packageTransformers.toList());
     });
 
     // If there are no assets or transformers, add a dummy package. This better
     // simulates the real world, where there'll always be at least the
     // entrypoint package.
-    if (_packages.isEmpty) _packages = {"app": new _MockPackage([], [])};
+    if (_packages.isEmpty) {
+      _packages = {"app": new _MockPackage(new AssetSet(), [])};
+    }
   }
 
   void _modifyAsset(String name, String contents) {
     var id = new AssetId.parse(name);
-    var asset = _packages[id.package].assets.firstWhere((a) => a.id == id);
-    asset.contents = contents;
+    _errors.remove(id);
+    _packages[id.package].assets[id].contents = contents;
   }
 
+  void _setAssetError(String name) => _errors.add(new AssetId.parse(name));

[Bob Nystrom, 2013/07/30 22:25:02]

Document.

[nweiz, 2013/07/30 22:46:21]

Like [_modifyAsset], I didn't document this because it's effectively already
documented above for [setAssetError].

+
   List<AssetId> listAssets(String package, {String within}) {
     if (within != null) {
       throw new UnimplementedError("Doesn't handle 'within' yet.");
@@ -323,6 +376,14 @@ class MockProvider implements PackageProvider {
   }
 
   Future<Asset> getAsset(AssetId id) {
+    // Eagerly load the asset so we can test an asset's value changing between
+    // when a load starts and when it finishes.
+    var package = _packages[id.package];
+    var asset;
+    if (package != null) asset = package.assets[id];
+
+    var error = _errors.contains(id);

[Bob Nystrom, 2013/07/30 22:25:02]

"hasError"

[nweiz, 2013/07/30 22:46:21]

Done.

+
     var future;
     if (_pauseCompleter != null) {
       future = _pauseCompleter.future;
@@ -331,225 +392,38 @@ class MockProvider implements PackageProvider {
     }
 
     return future.then((_) {
-      var package = _packages[id.package];
-      if (package == null) throw new AssetNotFoundException(id);
-
-      return package.assets.firstWhere((asset) => asset.id == id,
-          orElse: () => throw new AssetNotFoundException(id));
+      if (error) throw new MockLoadException(id);
+      if (asset == null) throw new AssetNotFoundException(id);
+      return asset;
     });
   }
 }
 
+/// Error thrown for assets with [setAssetError] set.
+class MockLoadException implements Exception {
+  final AssetId id;
+
+  MockLoadException(this.id);
+
+  String toString() => "Error loading $id.";
+}
+
 /// Used by [MockProvider] to keep track of which assets and transformers exist
 /// for each package.
 class _MockPackage {
-  final List<MockAsset> assets;
+  final AssetSet assets;
   final List<List<Transformer>> transformers;
 
   _MockPackage(this.assets, Iterable<Iterable<Transformer>> transformers)
       : transformers = transformers.map((phase) => phase.toList()).toList();
 }
 
-/// A [Transformer] that takes assets ending with one extension and generates
-/// assets with a given extension.
-///
-/// Appends the output extension to the contents of the input file.
-class RewriteTransformer extends Transformer {
-  final String from;
-  final String to;
-
-  /// The number of times the transformer has been applied.
-  int numRuns = 0;
-
-  /// The number of currently running transforms.
-  int _runningTransforms = 0;
-
-  /// The completer that the transform is waiting on to complete.
-  ///
-  /// If `null` the transform will complete immediately.
-  Completer _wait;
-
-  /// A future that completes when the first apply of this transformer begins.
-  Future get started => _started.future;
-  final _started = new Completer();
-
-  /// Creates a transformer that rewrites assets whose extension is [from] to
-  /// one whose extension is [to].
-  ///
-  /// [to] may be a space-separated list in which case multiple outputs will be
-  /// created for each input.
-  RewriteTransformer(this.from, this.to);
-
-  /// `true` if any transforms are currently running.
-  bool get isRunning => _runningTransforms > 0;
-
-  /// Tells the transform to wait during its transformation until [complete()]
-  /// is called.
-  ///
-  /// Lets you test the asynchronous behavior of transformers.
-  void wait() {
-    _wait = new Completer();
-  }
-
-  void complete() {
-    _wait.complete();
-    _wait = null;
-  }
-
-  Future<bool> isPrimary(Asset asset) {
-    return new Future.value(asset.id.extension == ".$from");
-  }
-
-  Future apply(Transform transform) {
-    numRuns++;
-    if (!_started.isCompleted) _started.complete();
-    _runningTransforms++;
-    return transform.primaryInput.then((input) {
-      return Future.wait(to.split(" ").map((extension) {
-        var id = transform.primaryId.changeExtension(".$extension");
-        return input.readAsString().then((content) {
-          transform.addOutput(new MockAsset(id, "$content.$extension"));
-        });
-      })).then((_) {
-        if (_wait != null) return _wait.future;
-      });
-    }).whenComplete(() {
-      _runningTransforms--;
-    });
-  }
-
-  String toString() => "$from->$to";
-}
-
-/// A [Transformer] that takes an input asset that contains a comma-separated
-/// list of paths and outputs a file for each path.
-class OneToManyTransformer extends Transformer {
-  final String extension;
-
-  /// The number of times the transformer has been applied.
-  int numRuns = 0;
-
-  /// Creates a transformer that consumes assets with [extension].
-  ///
-  /// That file contains a comma-separated list of paths and it will output
-  /// files at each of those paths.
-  OneToManyTransformer(this.extension);
-
-  Future<bool> isPrimary(Asset asset) {
-    return new Future.value(asset.id.extension == ".$extension");
-  }
-
-  Future apply(Transform transform) {
-    numRuns++;
-    return transform.primaryInput.then((input) {
-      return input.readAsString().then((lines) {
-        for (var line in lines.split(",")) {
-          var id = new AssetId(transform.primaryId.package, line);
-          transform.addOutput(new MockAsset(id, "spread $extension"));
-        }
-      });
-    });
-  }
-
-  String toString() => "1->many $extension";
-}
-
-/// A transformer that uses the contents of a file to define the other inputs.
-///
-/// Outputs a file with the same name as the primary but with an "out"
-/// extension containing the concatenated contents of all non-primary inputs.
-class ManyToOneTransformer extends Transformer {
-  final String extension;
-
-  /// The number of times the transformer has been applied.
-  int numRuns = 0;
-
-  /// Creates a transformer that consumes assets with [extension].
-  ///
-  /// That file contains a comma-separated list of paths and it will input
-  /// files at each of those paths.
-  ManyToOneTransformer(this.extension);
-
-  Future<bool> isPrimary(Asset asset) {
-    return new Future.value(asset.id.extension == ".$extension");
-  }
-
-  Future apply(Transform transform) {
-    numRuns++;
-    return transform.primaryInput.then((primary) {
-      return primary.readAsString().then((contents) {
-        // Get all of the included inputs.
-        var inputs = contents.split(",").map((path) {
-          var id = new AssetId(transform.primaryId.package, path);
-          return transform.getInput(id);
-        });
-
-        return Future.wait(inputs);
-      }).then((inputs) {
-        // Concatenate them to one output.
-        var output = "";
-        return Future.forEach(inputs, (input) {
-          return input.readAsString().then((contents) {
-            output += contents;
-          });
-        }).then((_) {
-          var id = transform.primaryId.changeExtension(".out");
-          transform.addOutput(new MockAsset(id, output));
-        });
-      });
-    });
-  }
-
-  String toString() => "many->1 $extension";
-}
-
-/// A transformer that throws an exception when run, after generating the
-/// given outputs.
-class BadTransformer extends Transformer {
-  /// The error it throws.
-  static const ERROR = "I am a bad transformer!";
-
-  /// The list of asset names that it should output.
-  final List<String> outputs;
-
-  BadTransformer(this.outputs);
-
-  Future<bool> isPrimary(Asset asset) => new Future.value(true);
-  Future apply(Transform transform) {
-    return newFuture(() {
-      // Create the outputs first.
-      for (var output in outputs) {
-        var id = new AssetId.parse(output);
-        transform.addOutput(new MockAsset(id, output));
-      }
-
-      // Then fail.
-      throw ERROR;
-    });
-  }
-}
-
-/// A transformer that outputs an asset with the given id.
-class CreateAssetTransformer extends Transformer {
-  final String output;
-
-  CreateAssetTransformer(this.output);
-
-  Future<bool> isPrimary(Asset asset) => new Future.value(true);
-
-  Future apply(Transform transform) {
-    return newFuture(() {
-      transform.addOutput(new MockAsset(new AssetId.parse(output), output));
-    });
-  }
-}
-
 /// An implementation of [Asset] that never hits the file system.
-class MockAsset implements Asset {
+class _MockAsset implements Asset {

[Bob Nystrom, 2013/07/30 22:25:02]

Can you get rid of this entirely?

[nweiz, 2013/07/30 22:46:21]

I considered that, but the fact that MockProvider directly sets [contents]
better emulates file assets, where the results of [readAsString] can
theoretically change from call to call. That shouldn't matter, but this is more
likely to fail if it does end up mattering.

[Bob Nystrom, 2013/07/30 23:14:08]

SGTM.

   final AssetId id;
   String contents;
 
-  MockAsset(this.id, this.contents);
+  _MockAsset(this.id, this.contents);
 
   Future<String> readAsString({Encoding encoding}) =>
       new Future.value(contents);
@@ -557,4 +431,4 @@ class MockAsset implements Asset {
   Stream<List<int>> read() => throw new UnimplementedError();
 
   String toString() => "MockAsset $id $contents";
-}
+}