Merge pull request #617 from garden-io/multiple-modules-watch-fix

fix: add tasks for all affected modules on watch

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 } },
     ])
   })
 })