Better handling of asset collisions in barback.

pkg/barback/test/package_graph/errors_test.dart

Index: pkg/barback/test/package_graph/errors_test.dart
diff --git a/pkg/barback/test/package_graph/errors_test.dart b/pkg/barback/test/package_graph/errors_test.dart
index 973155bdb709779dc2a5783e8b2ce69bb20f5c0d..885c9f3cb92e87e1c862e694b6e60c6792ce9510 100644
--- a/pkg/barback/test/package_graph/errors_test.dart
+++ b/pkg/barback/test/package_graph/errors_test.dart
@@ -7,6 +7,7 @@ library barback.test.package_graph.source_test;
 import 'dart:async';
 
 import 'package:barback/barback.dart';
+import 'package:barback/src/utils.dart';
 import 'package:scheduled_test/scheduled_test.dart';
 
 import '../utils.dart';
@@ -38,7 +39,7 @@ main() {
     expectAsset("app|foo.c", "foo.c");
     buildShouldSucceed();
 
-    schedule(() => updateSources(["app|foo.b"]));
+    updateSources(["app|foo.b"]);
     buildShouldFail([isAssetCollisionException("app|foo.c")]);
   });
 
@@ -54,7 +55,7 @@ main() {
 
   test("reports an error for an unprovided package", () {
     initGraph();
-    expect(() => updateSources(["unknown|foo.txt"]), throwsArgumentError);
+    expect(() => updateSourcesSync(["unknown|foo.txt"]), throwsArgumentError);
   });
 
   test("reports an error for an unprovided source", () {
@@ -69,11 +70,9 @@ main() {
       [new ManyToOneTransformer("txt")]
     ]});
 
-    buildShouldFail([isMissingInputException("app|a.inc")]);
-
     updateSources(["app|a.txt"]);
-
     expectNoAsset("app|a.out");
+    buildShouldFail([isMissingInputException("app|a.inc")]);
   });
 
   test("reports an error if a transformer emits an asset for another package",
@@ -82,9 +81,8 @@ main() {
       "app": [[new CreateAssetTransformer("wrong|foo.txt")]]
     });
 
-    buildShouldFail([isInvalidOutputException("app", "wrong|foo.txt")]);
-
     updateSources(["app|foo.txt"]);
+    buildShouldFail([isInvalidOutputException("app", "wrong|foo.txt")]);
   });
 
   test("fails if a non-primary input is removed", () {
@@ -101,10 +99,7 @@ main() {
     expectAsset("app|a.out", "abc");
     buildShouldSucceed();
 
-    schedule(() {
-      removeSources(["app|b.inc"]);
-    });
-
+    removeSources(["app|b.inc"]);
     buildShouldFail([isMissingInputException("app|b.inc")]);
     expectNoAsset("app|a.out");
   });
@@ -114,12 +109,8 @@ main() {
       [new BadTransformer(["app|foo.out"])]
     ]});
 
-    schedule(() {
-      updateSources(["app|foo.txt"]);
-    });
-
+    updateSources(["app|foo.txt"]);
     expectNoAsset("app|foo.out");
-
     buildShouldFail([equals(BadTransformer.ERROR)]);
   });
 
@@ -128,22 +119,15 @@ main() {
       [new BadTransformer(["app|foo.txt"])]
     ]});
 
-    schedule(() {
-      updateSources(["app|foo.txt"]);
-    });
-
+    updateSources(["app|foo.txt"]);
     expectNoAsset("app|foo.txt");
   });
 
   test("catches errors even if nothing is waiting for process results", () {
     initGraph(["app|foo.txt"], {"app": [[new BadTransformer([])]]});
 
-    schedule(() {
-      updateSources(["app|foo.txt"]);
-    });
-
+    updateSources(["app|foo.txt"]);
     // Note: No asset requests here.
-
     buildShouldFail([equals(BadTransformer.ERROR)]);
   });
 
@@ -152,10 +136,7 @@ main() {
       [new BadTransformer(["a.out", "b.out"])]
     ]});
 
-    schedule(() {
-      updateSources(["app|foo.txt"]);
-    });
-
+    updateSources(["app|foo.txt"]);
     expectNoAsset("app|a.out");
   });
 
@@ -163,10 +144,7 @@ main() {
     initGraph(["pkg1|foo.txt", "pkg2|foo.txt"],
         {"pkg1": [[new BadTransformer([])]]});
 
-    schedule(() {
-      updateSources(["pkg1|foo.txt", "pkg2|foo.txt"]);
-    });
-
+    updateSources(["pkg1|foo.txt", "pkg2|foo.txt"]);
     expectAsset("pkg2|foo.txt", "foo");
     buildShouldFail([equals(BadTransformer.ERROR)]);
   });
@@ -177,10 +155,7 @@ main() {
       "pkg2": [[new BadTransformer([])]]
     });
 
-    schedule(() {
-      updateSources(["pkg1|foo.txt", "pkg2|foo.txt"]);
-    });
-
+    updateSources(["pkg1|foo.txt", "pkg2|foo.txt"]);
     buildShouldFail([
       equals(BadTransformer.ERROR),
       equals(BadTransformer.ERROR)
@@ -191,8 +166,117 @@ main() {
     initGraph(["app|foo.txt"]);
 
     setAssetError("app|foo.txt");
-    schedule(() => updateSources(["app|foo.txt"]));
+    updateSources(["app|foo.txt"]);
     expectNoAsset("app|foo.txt");
     buildShouldFail([isMockLoadException("app|foo.txt")]);
   });
+
+  test("a collision returns the first-produced output", () {
+    var rewrite1 = new RewriteTransformer("one", "out");
+    var rewrite2 = new RewriteTransformer("two", "out");
+    initGraph({
+      "app|foo.one": "one",
+      "app|foo.two": "two"
+    }, {"app": [[rewrite1, rewrite2]]});
+
+    rewrite1.pauseApply();
+    updateSources(["app|foo.one", "app|foo.two"]);
+    // Wait long enough to ensure that rewrite2 has completed.
+    schedule(pumpEventQueue);
+
+    rewrite1.resumeApply();
+    expectAsset("app|foo.out", "two.out");
+    buildShouldFail([isAssetCollisionException("app|foo.out")]);
+
+    // Even after the collision is discovered, the first-produced output should
+    // be returned.
+    expectAsset("app|foo.out", "two.out");
+
+    // Even if the other output is updated more recently, the first output
+    // should continue to take precedence.
+    updateSources(["app|foo.one"]);
+    expectAsset("app|foo.out", "two.out");
+  });
+
+  test("a collision that is later resolved produces an output", () {
+    initGraph({
+      "app|foo.one": "one",
+      "app|foo.two": "two"
+    }, {"app": [
+      [
+        new RewriteTransformer("one", "out"),
+        new RewriteTransformer("two", "out")
+      ]
+    ]});
+
+    updateSources(["app|foo.one"]);
+    expectAsset("app|foo.out", "one.out");
+    buildShouldSucceed();
+
+    updateSources(["app|foo.two"]);
+    expectAsset("app|foo.out", "one.out");
+    buildShouldFail([isAssetCollisionException("app|foo.out")]);
+
+    removeSources(["app|foo.one"]);
+    expectAsset("app|foo.out", "two.out");
+    buildShouldSucceed();
+  });
+
+  test("a collision that is later resolved runs transforms", () {
+    initGraph({
+      "app|foo.one": "one",
+      "app|foo.two": "two"
+    }, {"app": [
+      [
+        new RewriteTransformer("one", "mid"),
+        new RewriteTransformer("two", "mid")
+      ],
+      [new RewriteTransformer("mid", "out")]
+    ]});
+
+    updateSources(["app|foo.one"]);
+    expectAsset("app|foo.out", "one.mid.out");
+    buildShouldSucceed();
+
+    updateSources(["app|foo.two"]);
+    expectAsset("app|foo.out", "one.mid.out");
+    buildShouldFail([isAssetCollisionException("app|foo.mid")]);
+
+    removeSources(["app|foo.one"]);
+    expectAsset("app|foo.out", "two.mid.out");
+    buildShouldSucceed();
+  });
+
+  test("a collision that is partially resolved returns the second completed "
+      "output", () {
+    var rewrite1 = new RewriteTransformer("one", "out");
+    var rewrite2 = new RewriteTransformer("two", "out");
+    var rewrite3 = new RewriteTransformer("three", "out");
+    initGraph({
+      "app|foo.one": "one",
+      "app|foo.two": "two",
+      "app|foo.three": "three"
+    }, {"app": [[rewrite1, rewrite2, rewrite3]]});
+
+    // Make rewrite3 the most-recently-completed transformer from the first run.
+    rewrite2.pauseApply();
+    rewrite3.pauseApply();
+    updateSources(["app|foo.one", "app|foo.two", "app|foo.three"]);
+    schedule(pumpEventQueue);
+    rewrite2.resumeApply();
+    schedule(pumpEventQueue);
+    rewrite3.resumeApply();
+    buildShouldFail([isAssetCollisionException("app|foo.out")]);
+
+    // Then update rewrite3 in a separate build. rewrite2 should still be the
+    // next version of foo.out in line.
+    // TODO(nweiz): Should this emit a collision error as well? Or should they
+    // only be emitted when a file is added or removed?
+    updateSources(["app|foo.three"]);
+    buildShouldSucceed();
+
+    removeSources(["app|foo.one"]);
+    expectAsset("app|foo.out", "two.out");
+    buildShouldFail([isAssetCollisionException("app|foo.out")]);
+  });

[Bob Nystrom, 2013/08/09 22:44:43]

Great tests!

[nweiz, 2013/08/12 21:15:44]

Thanks!

 }