fix: add tasks for all affected modules on watch

Before this commit, changing a file belonging to more than one module
(i.e. where both/all modules are defined in the same garden.yml) would
only add tasks for the first module defined in that garden.yml.

After this change, tasks are added for all modules in the file when
their shared sources change.

garden-service/src/events.ts

@@ -61,15 +61,14 @@ export interface Events {
   },
   projectConfigChanged: {},
   moduleConfigChanged: {
-    name: string,
+    names: string[],
     path: string,
   },
   moduleSourcesChanged: {
-    name: string,
+    names: string[],
     pathChanged: string,
   },
   moduleRemoved: {
-    name: string,
   },
 
   // TaskGraph events

garden-service/src/process.ts

@@ -146,26 +146,28 @@ export async function processModules(
 
     garden.events.on("moduleConfigChanged", async (event) => {
       if (await validateConfigChange(garden, log, event.path, "changed")) {
-        log.info({ symbol: "info", section: event.name, msg: `Module configuration changed, reloading...` })
+        const moduleNames = event.names
+        const section = moduleNames.length === 1 ? moduleNames[0] : undefined
+        log.info({ symbol: "info", section, msg: `Module configuration changed, reloading...` })
         resolve()
       }
     })
 
     garden.events.on("moduleSourcesChanged", async (event) => {
-      const changedModule = modulesByName[event.name]
+      graph = await garden.getConfigGraph()
+      const changedModuleNames = event.names.filter((moduleName) => !!modulesByName[moduleName])
 
-      if (!changedModule) {
+      if (changedModuleNames.length === 0) {
         return
       }
 
-      // Update the config graph
-      graph = await garden.getConfigGraph()
-
-      // Make sure the module's version is up to date.
-      const refreshedModule = await graph.getModule(changedModule.name)
-      modulesByName[event.name] = refreshedModule
+      // Make sure the modules' versions are up to date.
+      const changedModules = await graph.getModules(changedModuleNames)
 
-      await Bluebird.map(changeHandler!(graph, refreshedModule), (task) => garden.addTask(task))
+      await Bluebird.map(changedModules, async (m) => {
+        modulesByName[m.name] = m
+        return Bluebird.map(changeHandler!(graph, m), (task) => garden.addTask(task))
+      })
       await garden.processTasks()
     })
   })

garden-service/src/watch.ts

@@ -80,13 +80,16 @@ export class Watcher {
 
       const parsed = parse(path)
       const filename = parsed.base
-      const changedModule = modules.find(m => path.startsWith(m.path)) || null
+
+      // changedModules will only have more than one element when the changed path belongs to >= 2 modules.
+      const changedModules = modules.filter(m => path.startsWith(m.path))
+      const changedModuleNames = changedModules.map(m => m.name)
 
       if (filename === MODULE_CONFIG_FILENAME || filename === ".gitignore" || filename === ".gardenignore") {
         this.invalidateCached(modules)
 
-        if (changedModule) {
-          this.garden.events.emit("moduleConfigChanged", { name: changedModule.name, path })
+        if (changedModuleNames.length > 0) {
+          this.garden.events.emit("moduleConfigChanged", { names: changedModuleNames, path })
         } else if (filename === MODULE_CONFIG_FILENAME) {
           if (parsed.dir === this.garden.projectRoot) {
             this.garden.events.emit("projectConfigChanged", {})
@@ -102,9 +105,9 @@ export class Watcher {
         return
       }
 
-      if (changedModule) {
-        this.invalidateCached([changedModule])
-        this.garden.events.emit("moduleSourcesChanged", { name: changedModule.name, pathChanged: path })
+      if (changedModuleNames.length > 0) {
+        this.invalidateCached(changedModules)
+        this.garden.events.emit("moduleSourcesChanged", { names: changedModuleNames, pathChanged: path })
       }
     }
   }
@@ -146,11 +149,13 @@ export class Watcher {
             return
           }
 
-          const changedModule = modules.find(m => path.startsWith(m.path))
+          // changedModules will only have more than one element when the changed path belongs to >= 2 modules.
+          const changedModules = modules.filter(m => path.startsWith(m.path))
+          const changedModuleNames = changedModules.map(m => m.name)
 
-          if (changedModule) {
-            this.invalidateCached([changedModule])
-            this.garden.events.emit("moduleSourcesChanged", { name: changedModule.name, pathChanged: path })
+          if (changedModules.length > 0) {
+            this.invalidateCached(changedModules)
+            this.garden.events.emit("moduleSourcesChanged", { names: changedModuleNames, pathChanged: path })
           }
         })
     }
@@ -164,14 +169,19 @@ export class Watcher {
         if (module.path.startsWith(path)) {
           // at least one module's root dir was removed
           this.invalidateCached(modules)
-          this.garden.events.emit("moduleRemoved", { name: module.name })
+          this.garden.events.emit("moduleRemoved", {})
           return
         }
 
         if (path.startsWith(module.path)) {
-          // removed dir is a subdir of changedModule's root dir
-          this.invalidateCached([module])
-          this.garden.events.emit("moduleSourcesChanged", { name: module.name, pathChanged: path })
+          /*
+           * Removed dir is a subdir of changedModules' root dir.
+           * changedModules will only have more than one element when the changed path belongs to >= 2 modules.
+           */
+          const changedModules = modules.filter(m => path.startsWith(m.path))
+          const changedModuleNames = changedModules.map(m => m.name)
+          this.invalidateCached(changedModules)
+          this.garden.events.emit("moduleSourcesChanged", { names: changedModuleNames, pathChanged: path })
         }
       }
     }

garden-service/test/data/test-project-watch/double-module/foo.txt

@@ -0,0 +1 @@
+bar

garden-service/test/data/test-project-watch/double-module/garden.yml

@@ -0,0 +1,29 @@
+module:
+  name: module-b
+  type: test
+  services:
+    - name: service-b
+  build:
+    command: [echo, B]
+  tests:
+    - name: unit
+      command: [echo, OK]
+  tasks:
+    - name: task-b
+      command: [echo, OK]
+
+---
+
+module:
+  name: module-c
+  type: test
+  services:
+    - name: service-c
+  build:
+    command: [echo, C]
+  tests:
+    - name: unit
+      command: [echo, OK]
+  tasks:
+    - name: task-c
+      command: [echo, OK]

garden-service/test/src/watch.ts

@@ -7,11 +7,13 @@ import pEvent = require("p-event")
 describe("Watcher", () => {
   let garden: TestGarden
   let modulePath: string
+  let doubleModulePath: string
   let moduleContext: CacheContext
 
   before(async () => {
     garden = await makeTestGarden(resolve(dataDir, "test-project-watch"))
     modulePath = resolve(garden.projectRoot, "module-a")
+    doubleModulePath = resolve(garden.projectRoot, "double-module")
     moduleContext = pathToCacheContext(modulePath)
     await garden.startWatcher(await garden.getConfigGraph())
   })
@@ -36,7 +38,7 @@ describe("Watcher", () => {
     const path = resolve(modulePath, "garden.yml")
     emitEvent("change", path)
     expect(garden.events.log).to.eql([
-      { name: "moduleConfigChanged", payload: { name: "module-a", path } },
+      { name: "moduleConfigChanged", payload: { names: ["module-a"], path } },
     ])
   })
 
@@ -80,12 +82,24 @@ describe("Watcher", () => {
     ])
   })
 
-  it("should emit a moduleSourcesChanged event when a module file is changed", async () => {
-    const pathChanged = resolve(modulePath, "foo.txt")
-    emitEvent("change", pathChanged)
-    expect(garden.events.log).to.eql([
-      { name: "moduleSourcesChanged", payload: { name: "module-a", pathChanged } },
-    ])
+  context("should emit a moduleSourcesChanged event", () => {
+
+    it("containing the module's name when one of its files is changed", async () => {
+      const pathChanged = resolve(modulePath, "foo.txt")
+      emitEvent("change", pathChanged)
+      expect(garden.events.log).to.eql([
+        { name: "moduleSourcesChanged", payload: { names: ["module-a"], pathChanged } },
+      ])
+    })
+
+    it("containing both modules' names when a source file is changed for two co-located modules", async () => {
+      const pathChanged = resolve(doubleModulePath, "foo.txt")
+      emitEvent("change", pathChanged)
+      expect(garden.events.log).to.eql([
+        { name: "moduleSourcesChanged", payload: { names: ["module-b", "module-c"], pathChanged } },
+      ])
+    })
+
   })
 
   it("should clear a module's cache when a module file is changed", async () => {
@@ -105,7 +119,7 @@ describe("Watcher", () => {
     const pathChanged = resolve(modulePath, "subdir")
     emitEvent("addDir", pathChanged)
     expect(await waitForEvent("moduleSourcesChanged")).to.eql({
-      name: "module-a",
+      names: ["module-a"],
       pathChanged,
     })
   })
@@ -119,15 +133,15 @@ describe("Watcher", () => {
   it("should emit a moduleRemoved event if a directory containing a module is removed", async () => {
     emitEvent("unlinkDir", modulePath)
     expect(garden.events.log).to.eql([
-      { name: "moduleRemoved", payload: { name: "module-a" } },
+      { name: "moduleRemoved", payload: {} },
     ])
   })
 
   it("should emit a moduleSourcesChanged event if a directory within a module is removed", async () => {
     const pathChanged = resolve(modulePath, "subdir")
     emitEvent("unlinkDir", pathChanged)
     expect(garden.events.log).to.eql([
-      { name: "moduleSourcesChanged", payload: { name: "module-a", pathChanged } },
+      { name: "moduleSourcesChanged", payload: { names: ["module-a"], pathChanged } },
     ])
   })
 })