Add the ability to dynamically modify transforms to barback.

pkg/barback/test/package_graph/transform_test.dart

Index: pkg/barback/test/package_graph/transform_test.dart
diff --git a/pkg/barback/test/package_graph/transform_test.dart b/pkg/barback/test/package_graph/transform_test.dart
index ba14d16eb58f9a1dc85241e9af2691291d50fd6c..9248e578544d62de509f0e2ff707415cbce1722d 100644
--- a/pkg/barback/test/package_graph/transform_test.dart
+++ b/pkg/barback/test/package_graph/transform_test.dart
@@ -806,6 +806,202 @@ main() {
     buildShouldSucceed();
   });
 
+  group("added/removed transformers", () {

[Bob Nystrom, 2013/08/20 19:08:54]

Let's move this to a separate test suite. This one's getting pretty large.

[nweiz, 2013/08/20 21:39:17]

Done.

+    test("a new transformer is applied to a matching asset", () {
+      initGraph(["app|foo.blub"]);
+
+      updateSources(["app|foo.blub"]);
+      expectAsset("app|foo.blub", "foo");
+      buildShouldSucceed();
+
+      updateTransformers("app", [[new RewriteTransformer("blub", "blab")]]);
+      expectAsset("app|foo.blab", "foo.blab");
+      expectNoAsset("app|foo.blub");
+      buildShouldSucceed();
+    });
+
+    test("a new transformer is not applied to a non-matching asset", () {
+      initGraph(["app|foo.blub"]);
+
+      updateSources(["app|foo.blub"]);
+      expectAsset("app|foo.blub", "foo");
+      buildShouldSucceed();
+
+      updateTransformers("app", [[new RewriteTransformer("zip", "zap")]]);
+      expectAsset("app|foo.blub", "foo");
+      expectNoAsset("app|foo.zap");
+      buildShouldSucceed();
+    });
+
+    test("updateTransformers doesn't re-run an old transformer", () {

[Bob Nystrom, 2013/08/20 19:08:54]

What if the old transformer is updated to be in a different phase? Say you had:

[
  [rewrite "txt" -> "foo"],
  [rewriteInPlace "txt"]
]

And you update it to:

[
  [rewriteInPlace "txt"]
  [rewrite "txt" -> "foo"]
]

At that point, the transformers do need to be re-run since "txt"->"foo" now
needs to process the rewritten-in-place asset. Looking at the code, I think it
will do the right thing, but it would be good to test it.

Another one to consider is:


[
  [],
  [rewriteInPlace "txt"]
]

Updated to:

[
  [rewriteInPlace "txt"]
  [rewrite "txt" -> "foo"]
]

Here, rewrite "txt" -> "foo" hasn't changed phase, but it still needs to be
re-run.

[nweiz, 2013/08/20 21:39:17]

Barback sees that as two new transformers. A transformer is only considered to
be "the same" if it's in the same phase. I'll add a test, though.
Added.

+      var rewrite = new RewriteTransformer("blub", "blab");
+      initGraph(["app|foo.blub"], {"app": [[rewrite]]});
+
+      updateSources(["app|foo.blub"]);
+      expectAsset("app|foo.blab", "foo.blab");
+      expectNoAsset("app|foo.blub");
+      buildShouldSucceed();
+
+      updateTransformers("app", [[rewrite]]);
+      expectAsset("app|foo.blab", "foo.blab");
+      expectNoAsset("app|foo.blub");
+      buildShouldSucceed();
+
+      expect(rewrite.numRuns, completion(equals(1)));
+    });
+
+    test("a removed transformer is no longer applied", () {
+      initGraph(["app|foo.blub"], {"app": [
+        [new RewriteTransformer("blub", "blab")]
+      ]});
+
+      updateSources(["app|foo.blub"]);
+      expectAsset("app|foo.blab", "foo.blab");
+      expectNoAsset("app|foo.blub");
+      buildShouldSucceed();
+
+      updateTransformers("app", []);
+      expectAsset("app|foo.blub", "foo");
+      expectNoAsset("app|foo.blab");
+      buildShouldSucceed();
+    });
+
+    test("a new transformer is pipelined", () {
+      var rewrite1 = new RewriteTransformer("source", "phase1");
+      var rewrite3 = new RewriteTransformer("phase2", "phase3");
+      initGraph(["app|foo.source"], {"app": [
+        [rewrite1],
+        [rewrite3]
+      ]});
+
+      updateSources(["app|foo.source"]);
+      expectNoAsset("app|foo.phase3");
+      buildShouldSucceed();
+
+      updateTransformers("app", [
+        [rewrite1],
+        [new RewriteTransformer("phase1", "phase2")],
+        [rewrite3]
+      ]);
+      expectAsset("app|foo.phase3", "foo.phase1.phase2.phase3");
+      buildShouldSucceed();
+    });
+
+    test("a removed transformer is un-pipelined", () {
+      var rewrite1 = new RewriteTransformer("source", "phase1");
+      var rewrite3 = new RewriteTransformer("phase2", "phase3");
+      initGraph(["app|foo.source"], {"app": [
+        [rewrite1],
+        [new RewriteTransformer("phase1", "phase2")],
+        [rewrite3]
+      ]});
+
+      updateSources(["app|foo.source"]);
+      expectAsset("app|foo.phase3", "foo.phase1.phase2.phase3");
+      buildShouldSucceed();
+
+      updateTransformers("app", [[rewrite1], [rewrite3]]);
+      expectNoAsset("app|foo.phase3");
+      buildShouldSucceed();
+    });
+
+    test("a transformer is removed during isPrimary", () {
+      var rewrite = new RewriteTransformer("blub", "blab");
+      initGraph(["app|foo.blub"], {"app": [[rewrite]]});
+
+      rewrite.pauseIsPrimary("app|foo.blub");
+      updateSources(["app|foo.blub"]);
+      // Ensure we're waiting on [rewrite.isPrimary].
+      schedule(pumpEventQueue);
+
+      updateTransformers("app", []);
+      rewrite.resumeIsPrimary("app|foo.blub");
+      expectAsset("app|foo.blub", "foo");
+      expectNoAsset("app|foo.blab");
+      buildShouldSucceed();
+    });
+
+    test("a transformer is removed during apply", () {
+      var rewrite = new RewriteTransformer("blub", "blab");
+      initGraph(["app|foo.blub"], {"app": [[rewrite]]});
+
+      rewrite.pauseApply();
+      updateSources(["app|foo.blub"]);
+      // Ensure we're waiting on [rewrite.apply].
+      schedule(pumpEventQueue);
+
+      updateTransformers("app", []);
+      rewrite.resumeApply();
+      expectAsset("app|foo.blub", "foo");
+      expectNoAsset("app|foo.blab");
+      buildShouldSucceed();
+    });
+
+    test("a new transformer can see pass-through assets", () {
+      var rewrite = new RewriteTransformer("zip", "zap");
+      initGraph(["app|foo.blub"], {"app": [[rewrite]]});
+
+      updateSources(["app|foo.blub"]);
+      buildShouldSucceed();
+
+      updateTransformers("app", [
+        [rewrite],
+        [new RewriteTransformer("blub", "blab")]
+      ]);
+      expectAsset("app|foo.blab", "foo.blab");
+      expectNoAsset("app|foo.blub");
+      buildShouldSucceed();
+    });
+
+    test("a cross-package transform sees a new transformer in a new phase", () {
+      // TODO(nweiz): make this work.
+      return;

[Bob Nystrom, 2013/08/20 19:08:54]

I'd like this to be more visible. How about pulling this test into a separate
file and then marking that as failing in the status file?

[nweiz, 2013/08/20 21:39:17]

It won't be around for more than a week or two; I've got in-progress patches
working towards fixing it. Given that, I'd rather leave this here for now.

[Bob Nystrom, 2013/08/20 22:18:12]

SGTM.

+
+      var rewrite = new RewriteTransformer("inc", "inc");
+      initGraph({
+        "pkg1|foo.txt": "pkg2|foo.inc",
+        "pkg2|foo.inc": "foo"
+      }, {
+        "pkg1": [[new ManyToOneTransformer("txt")]],
+        "pkg2": [[rewrite]]
+      });
+
+      updateSources(["pkg1|foo.txt", "pkg2|foo.inc"]);
+      expectAsset("pkg1|foo.out", "foo");
+      buildShouldSucceed();
+
+      updateTransformers("pkg2", [
+        [rewrite],
+        [new RewriteTransformer("inc", "inc")]
+      ]);
+      expectAsset("pkg1|foo.out", "foo.inc.inc");
+      buildShouldSucceed();
+    });
+
+    test("a cross-package transform doesn't see a removed transformer in a "
+        "removed phase", () {
+      var rewrite = new RewriteTransformer("inc", "inc");
+      initGraph({
+        "pkg1|foo.txt": "pkg2|foo.inc",
+        "pkg2|foo.inc": "foo"
+      }, {
+        "pkg1": [[new ManyToOneTransformer("txt")]],
+        "pkg2": [
+          [rewrite],
+          [new RewriteTransformer("inc", "inc")]
+        ]
+      });
+
+      updateSources(["pkg1|foo.txt", "pkg2|foo.inc"]);
+      expectAsset("pkg1|foo.out", "foo.inc.inc");
+      buildShouldSucceed();
+
+      updateTransformers("pkg2", [[rewrite]]);
+      expectAsset("pkg1|foo.out", "foo.inc");
+      buildShouldSucceed();
+    });
+  });
+
   group("pass-through", () {
     test("passes an asset through a phase in which no transforms apply", () {
       initGraph([
@@ -1055,5 +1251,45 @@ main() {
       expectNoAsset("pkg1|c.done");
       buildShouldSucceed();
     });
+
+    test("sees a transformer that's newly applied to a cross-package "
+        "dependency", () {
+      initGraph({
+        "pkg1|a.txt": "pkg2|a.inc",
+        "pkg2|a.inc": "a"
+      }, {
+        "pkg1": [[new ManyToOneTransformer("txt")]],
+        "pkg2": [[new CheckContentTransformer("b", " transformed")]]
+      });
+
+      updateSources(["pkg1|a.txt", "pkg2|a.inc"]);
+      expectAsset("pkg1|a.out", "a");
+      buildShouldSucceed();
+
+      modifyAsset("pkg2|a.inc", "b");
+      updateSources(["pkg2|a.inc"]);
+      expectAsset("pkg1|a.out", "b transformed");
+      buildShouldSucceed();
+    });
+
+    test("doesn't see a transformer that's newly not applied to a "
+        "cross-package dependency", () {
+      initGraph({
+        "pkg1|a.txt": "pkg2|a.inc",
+        "pkg2|a.inc": "a"
+      }, {
+        "pkg1": [[new ManyToOneTransformer("txt")]],
+        "pkg2": [[new CheckContentTransformer("a", " transformed")]]
+      });
+
+      updateSources(["pkg1|a.txt", "pkg2|a.inc"]);
+      expectAsset("pkg1|a.out", "a transformed");
+      buildShouldSucceed();
+
+      modifyAsset("pkg2|a.inc", "b");
+      updateSources(["pkg2|a.inc"]);
+      expectAsset("pkg1|a.out", "b");
+      buildShouldSucceed();
+    });
   });
 }