From 4af27b10ae188e3320ba599200f6be336bc8dc93 Mon Sep 17 00:00:00 2001
From: Thorarinn Sigurdsson <thorarinnsigurdsson@gmail.com>
Date: Mon, 27 Jan 2020 17:08:55 +0100
Subject: [PATCH] fix(core): make test task deps hot-reload aware

Before this fix, the getDependencies method of TestTask returned
deploy tasks without the hot reload flag set appropriately.

This meant that repeated hot reloads of services with test dependants
would fail (because the test dependant would redeploy the service
without hot reloading enabled) when using the dev command.
---
 garden-service/src/commands/dev.ts         |  1 +
 garden-service/src/tasks/test.ts           | 20 ++++++++++++++++----
 garden-service/test/unit/src/tasks/test.ts | 22 +++++++++++++++++++++-
 3 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/garden-service/src/commands/dev.ts b/garden-service/src/commands/dev.ts
index 738509e1f4..a24c4b56c7 100644
--- a/garden-service/src/commands/dev.ts
+++ b/garden-service/src/commands/dev.ts
@@ -204,6 +204,7 @@ export class DevCommand extends Command<DevCommandArgs, DevCommandOpts> {
                   module: m,
                   graph: updatedGraph,
                   filterNames,
+                  hotReloadServiceNames,
                 })
               )
             )
diff --git a/garden-service/src/tasks/test.ts b/garden-service/src/tasks/test.ts
index 7691e60d3b..38aa397741 100644
--- a/garden-service/src/tasks/test.ts
+++ b/garden-service/src/tasks/test.ts
@@ -8,7 +8,7 @@
 
 import Bluebird from "bluebird"
 import chalk from "chalk"
-import { find } from "lodash"
+import { find, includes } from "lodash"
 import minimatch = require("minimatch")
 
 import { Module } from "../types/module"
@@ -40,6 +40,7 @@ export interface TestTaskParams {
   testConfig: TestConfig
   force: boolean
   forceBuild: boolean
+  hotReloadServiceNames?: string[]
 }
 
 export class TestTask extends BaseTask {
@@ -49,6 +50,7 @@ export class TestTask extends BaseTask {
   private graph: ConfigGraph
   private testConfig: TestConfig
   private forceBuild: boolean
+  private hotReloadServiceNames: string[]
 
   constructor({
     garden,
@@ -59,6 +61,7 @@ export class TestTask extends BaseTask {
     force,
     forceBuild,
     version,
+    hotReloadServiceNames = [],
   }: TestTaskParams & TaskParams & { _guard: true }) {
     // Note: The _guard attribute is to prevent accidentally bypassing the factory method
     super({ garden, log, force, version })
@@ -67,6 +70,7 @@ export class TestTask extends BaseTask {
     this.testConfig = testConfig
     this.force = force
     this.forceBuild = forceBuild
+    this.hotReloadServiceNames = hotReloadServiceNames
   }
 
   static async factory(initArgs: TestTaskParams): Promise<TestTask> {
@@ -83,7 +87,12 @@ export class TestTask extends BaseTask {
     }
 
     const dg = this.graph
-    const deps = await dg.getDependencies({ nodeType: "test", name: this.getName(), recursive: false })
+    const deps = await dg.getDependencies({
+      nodeType: "test",
+      name: this.getName(),
+      recursive: false,
+      filterFn: (depNode) => !(depNode.type === "deploy" && includes(this.hotReloadServiceNames, depNode.name)),
+    })
 
     const buildTasks = await BuildTask.factory({
       garden: this.garden,
@@ -103,7 +112,7 @@ export class TestTask extends BaseTask {
       })
     })
 
-    const serviceTasks = deps.deploy.map(
+    const deployTasks = deps.deploy.map(
       (service) =>
         new DeployTask({
           garden: this.garden,
@@ -115,7 +124,7 @@ export class TestTask extends BaseTask {
         })
     )
 
-    return [...buildTasks, ...serviceTasks, ...taskTasks]
+    return [...buildTasks, ...deployTasks, ...taskTasks]
   }
 
   getName() {
@@ -220,6 +229,7 @@ export async function getTestTasks({
   graph,
   module,
   filterNames,
+  hotReloadServiceNames,
   force = false,
   forceBuild = false,
 }: {
@@ -228,6 +238,7 @@ export async function getTestTasks({
   graph: ConfigGraph
   module: Module
   filterNames?: string[]
+  hotReloadServiceNames?: string[]
   force?: boolean
   forceBuild?: boolean
 }) {
@@ -248,6 +259,7 @@ export async function getTestTasks({
       forceBuild,
       testConfig: test,
       module,
+      hotReloadServiceNames,
     })
   )
 }
diff --git a/garden-service/test/unit/src/tasks/test.ts b/garden-service/test/unit/src/tasks/test.ts
index f4e13c23f4..1fe80ed03b 100644
--- a/garden-service/test/unit/src/tasks/test.ts
+++ b/garden-service/test/unit/src/tasks/test.ts
@@ -8,7 +8,7 @@
 
 import { expect } from "chai"
 import { resolve } from "path"
-import { TestTask } from "../../../../src/tasks/test"
+import { TestTask, getTestTasks } from "../../../../src/tasks/test"
 import td from "testdouble"
 import { Garden } from "../../../../src/garden"
 import { dataDir, makeTestGarden } from "../../../helpers"
@@ -96,4 +96,24 @@ describe("TestTask", () => {
       expect(deps.map((d) => d.getKey())).to.eql(["build.module-a", "deploy.service-b", "task.task-a"])
     })
   })
+
+  describe("getTestTasks", () => {
+    it("should not return test tasks with deploy dependencies on services deployed with hot reloading", async () => {
+      const moduleA = await graph.getModule("module-a")
+
+      const tasks = await getTestTasks({
+        garden,
+        log,
+        graph,
+        module: moduleA,
+        hotReloadServiceNames: ["service-b"],
+      })
+
+      const testTask = tasks[0]
+      const deps = await testTask.getDependencies()
+
+      expect(tasks.length).to.eql(1)
+      expect(deps.map((d) => d.getKey())).to.eql(["build.module-a", "task.task-a"])
+    })
+  })
 })