fix(task-graph): don't re-run failed tasks

After this change, a task will not be re-run if it has force = false and
another task with the same key and version has already thrown an error
while running (and thus had its result cached in the graph's
`ResultCache`).

This means that we treat task results as cached regardless of whether
the task succeeded or not: The semantic is that a task with force =
false should always succeed with the same result (or fail) for a given
key + version.

When this is not the case, force = true should be used.

This prevents e.g. time-consuming tasks from repeatedly running and
failing in the same fashion when repeatedly added to the task graph
following watch changes.

garden-service/src/task-graph.ts

@@ -195,7 +195,7 @@ export class TaskGraph {
     } else {
       const result = this.resultCache.get(node.getKey(), node.getVersion())
       if (result) {
-        this.garden.events.emit("taskComplete", result)
+        this.garden.events.emit(result.error ? "taskError" : "taskComplete", result)
       }
     }
   }
@@ -733,12 +733,12 @@ class ResultCache {
 
   get(key: string, versionString: string): TaskResult | null {
     const r = this.cache[key]
-    return r && r.versionString === versionString && !r.result.error ? r.result : null
+    return r && r.versionString === versionString ? r.result : null
   }
 
   getNewest(key: string): TaskResult | null {
     const r = this.cache[key]
-    return r && !r.result.error ? r.result : null
+    return r ? r.result : null
   }
 
   // Returns newest cached results, if any, for keys

garden-service/test/unit/src/task-graph.ts

@@ -275,6 +275,96 @@ describe("task-graph", () => {
       expect(results.c!.error).to.not.exist
     })
 
+    context("if a successful result has been cached", () => {
+      it("should not process a matching task with force = false", async () => {
+        const garden = await getGarden()
+        const graph = new TaskGraph(garden, garden.log)
+        const resultCache = graph["resultCache"]
+
+        const resultOrder: string[] = []
+        const callback = async (key: string, _result: any) => {
+          resultOrder.push(key)
+        }
+        const opts = { callback }
+
+        const task = new TestTask(garden, "a", false, { ...opts, uid: "a1", versionString: "1" })
+        await graph.process([task])
+        expect(resultCache.get("a", "1")).to.exist
+
+        const repeatedTask = new TestTask(garden, "a", false, { ...opts, uid: "a2", versionString: "1" })
+        await graph.process([repeatedTask])
+
+        expect(resultOrder).to.eql(["a.a1"])
+      })
+
+      it("should process a matching task with force = true", async () => {
+        const garden = await getGarden()
+        const graph = new TaskGraph(garden, garden.log)
+
+        const resultOrder: string[] = []
+        const callback = async (key: string, _result: any) => {
+          resultOrder.push(key)
+        }
+        const opts = { callback }
+
+        const task = new TestTask(garden, "a", true, { ...opts, uid: "a1", versionString: "1" })
+        await graph.process([task])
+
+        const repeatedTask = new TestTask(garden, "a", true, { ...opts, uid: "a2", versionString: "1" })
+        await graph.process([repeatedTask])
+
+        expect(resultOrder).to.eql(["a.a1", "a.a2"])
+      })
+    })
+
+    context("if a failing result has been cached", () => {
+      it("should not process a matching task with force = false", async () => {
+        const garden = await getGarden()
+        const graph = new TaskGraph(garden, garden.log)
+        const resultCache = graph["resultCache"]
+
+        const resultOrder: string[] = []
+        const callback = async (key: string, _result: any) => {
+          resultOrder.push(key)
+        }
+        const opts = { callback }
+
+        const task = new TestTask(garden, "a", false, { ...opts, uid: "a1", versionString: "1", throwError: true })
+        try {
+          await graph.process([task])
+        } catch (error) {}
+
+        const cacheEntry = resultCache.get("a", "1")
+        expect(cacheEntry, "expected cache entry to exist").to.exist
+
+        const repeatedTask = new TestTask(garden, "a", false, { ...opts, uid: "a2", versionString: "1" })
+        await graph.process([repeatedTask])
+
+        expect(resultOrder).to.eql(["a.a1"])
+      })
+
+      it("should process a matching task with force = true", async () => {
+        const garden = await getGarden()
+        const graph = new TaskGraph(garden, garden.log)
+
+        const resultOrder: string[] = []
+        const callback = async (key: string, _result: any) => {
+          resultOrder.push(key)
+        }
+        const opts = { callback }
+
+        const task = new TestTask(garden, "a", false, { ...opts, uid: "a1", versionString: "1", throwError: true })
+        try {
+          await graph.process([task])
+        } catch (error) {}
+
+        const repeatedTask = new TestTask(garden, "a", true, { ...opts, uid: "a2", versionString: "1" })
+        await graph.process([repeatedTask])
+
+        expect(resultOrder).to.eql(["a.a1", "a.a2"])
+      })
+    })
+
     it("should process multiple tasks in dependency order", async () => {
       const now = freezeTime()
       const garden = await getGarden()