fix: added tests + fixed subtle issues in getModuleWatchTasks (TBS)

garden-service/src/commands/deploy.ts

@@ -167,7 +167,6 @@ export class DeployCommand extends Command<Args, Opts> {
           graph,
           log,
           module,
-          serviceNames: module.serviceNames.filter((name) => serviceNames.includes(name)),
           hotReloadServiceNames,
         })
 

garden-service/src/commands/dev.ts

@@ -189,7 +189,6 @@ export class DevCommand extends Command<DevCommandArgs, DevCommandOpts> {
           log,
           graph: updatedGraph,
           module,
-          serviceNames: module.serviceNames,
           hotReloadServiceNames,
         })
 

garden-service/src/config-graph.ts

@@ -362,24 +362,20 @@ export class ConfigGraph {
   /**
    * Returns the set union of modules with the set union of their dependants (across all dependency types, recursively).
    */
-  async withDependantModules(modules: Module[], filterFn?: DependencyRelationFilterFn): Promise<Module[]> {
-    const dependants = flatten(await Bluebird.map(modules, (m) => this.getDependantsForModule(m, filterFn)))
+  async withDependantModules(modules: Module[]): Promise<Module[]> {
+    const dependants = flatten(await Bluebird.map(modules, (m) => this.getDependantsForModule(m, true)))
     // We call getModules to ensure that the returned modules have up-to-date versions.
     const dependantModules = await this.modulesForRelations(await this.mergeRelations(...dependants))
     return this.getModules({ names: uniq(modules.concat(dependantModules).map((m) => m.name)), includeDisabled: true })
   }
 
   /**
-   * Returns all build and runtime dependants of module and its services & tasks (recursively).
+   * Returns all build and runtime dependants of a module and its services & tasks (recursively).
+   * Includes the services and tasks contained in the given module, but does _not_ contain the build node for the
+   * module itself.
    */
-  async getDependantsForModule(module: Module, filterFn?: DependencyRelationFilterFn): Promise<DependencyRelations> {
-    return this.mergeRelations(
-      ...(await Bluebird.all([
-        this.getDependants({ nodeType: "build", name: module.name, recursive: true, filterFn }),
-        this.getDependantsForMany({ nodeType: "deploy", names: module.serviceNames, recursive: true, filterFn }),
-        this.getDependantsForMany({ nodeType: "run", names: module.taskNames, recursive: true, filterFn }),
-      ]))
-    )
+  async getDependantsForModule(module: Module, recursive: boolean): Promise<DependencyRelations> {
+    return this.getDependants({ nodeType: "build", name: module.name, recursive })
   }
 
   /**
@@ -535,8 +531,8 @@ export class ConfigGraph {
   private async relationsFromNames(names: DependencyRelationNames): Promise<DependencyRelations> {
     return Bluebird.props({
       build: this.getModules({ names: names.build, includeDisabled: true }),
-      deploy: this.getServices({ names: names.deploy }),
-      run: this.getTasks({ names: names.run }),
+      deploy: this.getServices({ names: names.deploy, includeDisabled: true }),
+      run: this.getTasks({ names: names.run, includeDisabled: true }),
       test: Object.values(pick(this.testConfigs, names.test)).map((t) => t.config),
     })
   }

garden-service/src/tasks/helpers.ts

@@ -7,7 +7,7 @@
  */
 
 import Bluebird from "bluebird"
-import { intersection, uniq, flatten } from "lodash"
+import { intersection, uniq, flatten, uniqBy } from "lodash"
 import { DeployTask } from "./deploy"
 import { Garden } from "../garden"
 import { Module } from "../types/module"
@@ -27,91 +27,70 @@ export async function getModuleWatchTasks({
   log,
   graph,
   module,
-  serviceNames,
   hotReloadServiceNames,
 }: {
   garden: Garden
   log: LogEntry
   graph: ConfigGraph
   module: Module
-  serviceNames: string[]
   hotReloadServiceNames: string[]
 }): Promise<BaseTask[]> {
   let buildTasks: BaseTask[] = []
-  let dependantBuildModules: Module[] = []
-  let servicesToDeploy: Service[] = []
 
-  const hotReloadModuleNames = await getModuleNames(graph, hotReloadServiceNames)
-
-  const dependantFilterFn = (dependantNode: DependencyGraphNode) =>
-    !hotReloadModuleNames.includes(dependantNode.moduleName)
-
-  if (intersection(module.serviceNames, hotReloadServiceNames).length) {
-    // Hot reloading is enabled for one or more of module's services.
-    const serviceDeps = await graph.getDependantsForMany({
-      nodeType: "deploy",
-      names: module.serviceNames,
-      recursive: true,
-      filterFn: dependantFilterFn,
-    })
-
-    dependantBuildModules = serviceDeps.build
-    servicesToDeploy = serviceDeps.deploy
-  } else {
-    const dependants = await graph.getDependantsForModule(module, dependantFilterFn)
+  const dependants = await graph.getDependantsForModule(module, true)
 
+  if (intersection(module.serviceNames, hotReloadServiceNames).length === 0) {
     buildTasks = await BuildTask.factory({
       garden,
       log,
       module,
       force: true,
     })
-
-    dependantBuildModules = dependants.build
-    servicesToDeploy = (await graph.getServices({ names: serviceNames })).concat(dependants.deploy)
   }
 
   const dependantBuildTasks = flatten(
-    await Bluebird.map(dependantBuildModules, (m) =>
-      BuildTask.factory({
-        garden,
-        log,
-        module: m,
-        force: false,
-      })
+    await Bluebird.map(
+      dependants.build.filter((m) => !m.disabled),
+      (m) =>
+        BuildTask.factory({
+          garden,
+          log,
+          module: m,
+          force: false,
+        })
     )
   )
 
-  const deployTasks = servicesToDeploy.map(
-    (service) =>
-      new DeployTask({
-        garden,
-        log,
-        graph,
-        service,
-        force: true,
-        forceBuild: false,
-        fromWatch: true,
-        hotReloadServiceNames,
-      })
-  )
+  const deployTasks = dependants.deploy
+    .filter((s) => !s.disabled && !hotReloadServiceNames.includes(s.name))
+    .map(
+      (service) =>
+        new DeployTask({
+          garden,
+          log,
+          graph,
+          service,
+          force: true,
+          forceBuild: false,
+          fromWatch: true,
+          hotReloadServiceNames,
+        })
+    )
 
-  const hotReloadServices = await graph.getServices({ names: hotReloadServiceNames })
+  const hotReloadServices = await graph.getServices({ names: hotReloadServiceNames, includeDisabled: true })
   const hotReloadTasks = hotReloadServices
-    .filter((service) => service.module.name === module.name || service.sourceModule.name === module.name)
+    .filter(
+      (service) =>
+        !service.disabled && (service.module.name === module.name || service.sourceModule.name === module.name)
+    )
     .map((service) => new HotReloadTask({ garden, graph, log, service, force: true }))
 
   const outputTasks = [...buildTasks, ...dependantBuildTasks, ...deployTasks, ...hotReloadTasks]
 
   log.silly(`getModuleWatchTasks called for module ${module.name}, returning the following tasks:`)
   log.silly(`  ${outputTasks.map((t) => t.getKey()).join(", ")}`)
 
-  return outputTasks
-}
-
-async function getModuleNames(dg: ConfigGraph, hotReloadServiceNames: string[]) {
-  const services = await dg.getServices({ names: hotReloadServiceNames })
-  return uniq(services.map((s) => s.module.name))
+  return uniqBy(outputTasks, (t) => t.getKey())
 }
 
 export function makeTestTaskName(moduleName: string, testConfigName: string) {