Clean up barback tests.

pkg/barback/test/asset_graph/errors_test.dart

Index: pkg/barback/test/asset_graph/errors_test.dart
diff --git a/pkg/barback/test/asset_graph/errors_test.dart b/pkg/barback/test/asset_graph/errors_test.dart
index 45a36de2abca3e48fe3c5fdded9d97df23da69d7..cd23e128551eb7c35c6594d5ac051b2d8aa1e4d0 100644
--- a/pkg/barback/test/asset_graph/errors_test.dart
+++ b/pkg/barback/test/asset_graph/errors_test.dart
@@ -16,205 +16,132 @@ main() {
   initConfig();
 
   test("errors if two transformers output the same file", () {
-    var provider = new MockProvider(["app|foo.a"]);
-    var graph = new AssetGraph(provider, [
+    initGraph(["app|foo.a"], [
       [
         new RewriteTransformer("a", "b"),
         new RewriteTransformer("a", "b")
       ]
     ]);
-    graph.updateSources([new AssetId.parse("app|foo.a")]);
+    updateSources(["app|foo.a"]);
 
-    expectCollision(graph, "app|foo.b");
+    expectCollision("app|foo.b");
   });
 
   test("reports asset not found errors in results", () {
-    var provider = new MockProvider([]);
-    var graph = new AssetGraph(provider, []);
-
-    // TODO(rnystrom): This is verbose and ugly. Better would be to have
-    // utils.dart register this on the graph and then have functions to expect
-    // certain build results.
-    var numResults = 0;
-    var gotError = false;
-    graph.results.listen(wrapAsync((result) {
-      numResults++;
-      expect(numResults, lessThan(3));
-
-      if (numResults == 1) {
-        // Should complete the build first.
-        expect(result.error, isNull);
-      } else if (numResults == 2) {
-        // Then have the error.
-        expect(result.error, new isInstanceOf<AssetNotFoundException>());
-        expect(result.error.id, equals(new AssetId.parse("app|foo.txt")));
-        gotError = true;
-      }
-    }));
-
-    expectNoAsset(graph, "app|foo.txt");
+    initGraph();
 
-    schedule(() {
-      expect(gotError, isTrue);
+    expectNoAsset("app|foo.txt");
+
+    buildShouldSucceed(); // TODO(rnystrom): Is this correct? Why?
+    buildShouldFail((error) {
+      expect(error, new isInstanceOf<AssetNotFoundException>());
+      expect(error.id, equals(new AssetId.parse("app|foo.txt")));
     });

[nweiz, 2013/06/25 22:37:56]

It definitely doesn't seem correct for a single build to emit both a success and
a failure result. It seems like if a call to [getAsset] fails due to the asset
not being found, that failure is local to the [getAsset] call; it shouldn't be
emitted through [results]. [results] should only emit an
[AssetNotFoundException] if some transformer tried to request an asset that
didn't exist.

[Bob Nystrom, 2013/06/26 20:44:45]

It's more that it emits a success *then* a failure result. The success is
because the build does complete. Then the request fails which also gets emitted.
OK, done. I think the last time we discussed this we'd decided that all errors
get piped to the results, but I'm agnostic.
There's a slightly different error for that (MissingInputException) but sounds
good.

   });
 
   test("reports an error for an unprovided source", () {
-    var provider = new MockProvider([]);
-    var graph = new AssetGraph(provider, []);
-    var resultFuture = graph.results.first;
-
-    graph.updateSources([new AssetId.parse("app|unknown.txt")]);
+    initGraph();
+    updateSources(["app|unknown.txt"]);
 
-    schedule(() {
-      return resultFuture.then((result) {
-        expect(result.error, new isInstanceOf<AssetNotFoundException>());
-        expect(result.error.id, equals(new AssetId.parse("app|unknown.txt")));
-      });
+    buildShouldFail((error) {
+      expect(error, new isInstanceOf<AssetNotFoundException>());
+      expect(error.id, equals(new AssetId.parse("app|unknown.txt")));
     });
   });
 
   test("reports missing input errors in results", () {
-    var provider = new MockProvider({"app|a.txt": "a.inc"});
-
-    var graph = new AssetGraph(provider, [
+    initGraph({"app|a.txt": "a.inc"}, [
       [new ManyToOneTransformer("txt")]
     ]);
 
-    var gotError = false;
-    graph.results.listen(wrapAsync((result) {
-      expect(result.error is MissingInputException, isTrue);
-      expect(result.error.id, equals(new AssetId.parse("app|a.inc")));
-      gotError = true;
-    }));
-
-    graph.updateSources([new AssetId.parse("app|a.txt")]);
+    buildShouldFail((error) {
+      expect(error, new isInstanceOf<MissingInputException>());
+      expect(error.id, equals(new AssetId.parse("app|a.inc")));
+    });
 
-    expectNoAsset(graph, "app|a.out");
+    updateSources(["app|a.txt"]);
 
-    schedule(() {
-      expect(gotError, isTrue);
-    });
+    expectNoAsset("app|a.out");
   });
 
   test("fails if a non-primary input is removed", () {
-    var provider = new MockProvider({
+    initGraph({
       "app|a.txt": "a.inc,b.inc,c.inc",
       "app|a.inc": "a",
       "app|b.inc": "b",
       "app|c.inc": "c"
-    });
-
-    var graph = new AssetGraph(provider, [
+    }, [
       [new ManyToOneTransformer("txt")]
     ]);
 
-    // TODO(rnystrom): This is verbose and ugly. Better would be to have
-    // utils.dart register this on the graph and then have functions to expect
-    // certain build results.
-    var numResults = 0;
-    var gotError = false;
-    graph.results.listen(wrapAsync((result) {
-      numResults++;
-      expect(numResults, lessThan(3));
-
-      if (numResults == 1) {
-        // Should complete the build first.
-        expect(result.error, isNull);
-      } else if (numResults == 2) {
-        // Then have the error.
-        expect(result.error is MissingInputException, isTrue);
-        expect(result.error.id, equals(new AssetId.parse("app|b.inc")));
-        gotError = true;
-      }
-    }));
-
-    graph.updateSources([
-      new AssetId.parse("app|a.txt"),
-      new AssetId.parse("app|a.inc"),
-      new AssetId.parse("app|b.inc"),
-      new AssetId.parse("app|c.inc")
-    ]);
-
-    expectAsset(graph, "app|a.out", "abc");
+    updateSources(["app|a.txt", "app|a.inc", "app|b.inc", "app|c.inc"]);
+    expectAsset("app|a.out", "abc");
+    buildShouldSucceed();
 
     schedule(() {
-      graph.removeSources([new AssetId.parse("app|b.inc")]);
+      removeSources(["app|b.inc"]);
     });
 
-    expectNoAsset(graph, "app|a.out");
-
-    schedule(() {
-      expect(gotError, isTrue);
+    buildShouldFail((error) {
+      expect(error, new isInstanceOf<MissingInputException>());
+      expect(error.id, equals(new AssetId.parse("app|b.inc")));
     });
+    expectNoAsset("app|a.out");
   });
 
   test("catches transformer exceptions and reports them", () {
-    var provider = new MockProvider(["app|foo.txt"]);
-    var graph = new AssetGraph(provider, [
+    initGraph(["app|foo.txt"], [
       [new BadTransformer(["app|foo.out"])]
     ]);
 
-    var gotError = false;
-    graph.results.listen(wrapAsync((result) {
-      expect(result.error, equals(BadTransformer.ERROR));
-      gotError = true;
-    }));
-
     schedule(() {
-      graph.updateSources([new AssetId.parse("app|foo.txt")]);
+      updateSources(["app|foo.txt"]);
     });
 
-    expectNoAsset(graph, "app|foo.out");
+    expectNoAsset("app|foo.out");
 
-    schedule(() {
-      expect(gotError, isTrue);
+    buildShouldFail((error) {
+      expect(error, equals(BadTransformer.ERROR));
     });
   });
 
   // TODO(rnystrom): Is this the behavior we expect? If a transformer fails
   // to transform a file, should we just skip past it to the source?
   test("yields a source if a transform fails on it", () {
-    var provider = new MockProvider(["app|foo.txt"]);
-    var graph = new AssetGraph(provider, [
+    initGraph(["app|foo.txt"], [
       [new BadTransformer(["app|foo.txt"])]
     ]);
 
     schedule(() {
-      graph.updateSources([new AssetId.parse("app|foo.txt")]);
+      updateSources(["app|foo.txt"]);
     });
 
-    expectAsset(graph, "app|foo.txt");
+    expectAsset("app|foo.txt");
   });
 
   test("catches errors even if nothing is waiting for process results", () {
-    var provider = new MockProvider(["app|foo.txt"]);
-    var graph = new AssetGraph(provider, [[new BadTransformer([])]]);
-    var resultFuture = graph.results.first;
+    initGraph(["app|foo.txt"], [[new BadTransformer([])]]);
 
     schedule(() {
-      graph.updateSources([new AssetId.parse("app|foo.txt")]);
+      updateSources(["app|foo.txt"]);
     });
 
     // Note: No asset requests here.
 
-    schedule(() {
-      return resultFuture.then((result) {
-        expect(result.error, equals(BadTransformer.ERROR));
-      });
+    buildShouldFail((error) {
+      expect(error, equals(BadTransformer.ERROR));
     });
   });
 
   test("discards outputs from failed transforms", () {
-    var provider = new MockProvider(["app|foo.txt"]);
-    var graph = new AssetGraph(provider, [
+    initGraph(["app|foo.txt"], [
       [new BadTransformer(["a.out", "b.out"])]
     ]);
 
     schedule(() {
-      graph.updateSources([new AssetId.parse("app|foo.txt")]);
+      updateSources(["app|foo.txt"]);
     });
 
-    expectNoAsset(graph, "app|a.out");
+    expectNoAsset("app|a.out");
   });
 }