Add check to extension file watch (#1677)

* Add check to extension file watch

Signed-off-by: Panu Horsmalahti <phorsmalahti@mirantis.com>

* Fix tests

Signed-off-by: Panu Horsmalahti <phorsmalahti@mirantis.com>

* Fix tests on Windows.

Signed-off-by: Panu Horsmalahti <phorsmalahti@mirantis.com>

* Add logging for Windows test debugging purposes.

Signed-off-by: Panu Horsmalahti <phorsmalahti@mirantis.com>

* Try to fix tests on Windows again.

Signed-off-by: Panu Horsmalahti <phorsmalahti@mirantis.com>

src/extensions/__tests__/extension-discovery.test.ts

@@ -0,0 +1,100 @@
+import { watch } from "chokidar";
+import { join, normalize } from "path";
+import { ExtensionDiscovery, InstalledExtension } from "../extension-discovery";
+
+jest.mock("../../common/ipc");
+jest.mock("fs-extra");
+jest.mock("chokidar", () => ({
+  watch: jest.fn()
+}));
+jest.mock("../extension-installer", () => ({
+  extensionInstaller: {
+    extensionPackagesRoot: "",
+    installPackages: jest.fn()
+  }
+}));
+
+const mockedWatch = watch as jest.MockedFunction<typeof watch>;
+
+describe("ExtensionDiscovery", () => {
+  it("emits add for added extension", async done => {
+    globalThis.__non_webpack_require__.mockImplementationOnce(() => ({
+      name: "my-extension"
+    }));
+    let addHandler: (filePath: string) => void;
+
+    const mockWatchInstance: any = {
+      on: jest.fn((event: string, handler: typeof addHandler) => {
+        if (event === "add") {
+          addHandler = handler;
+        }
+
+        return mockWatchInstance;
+      })
+    };
+
+    mockedWatch.mockImplementationOnce(() =>
+      (mockWatchInstance) as any
+    );
+    const extensionDiscovery = new ExtensionDiscovery();
+
+    // Need to force isLoaded to be true so that the file watching is started
+    extensionDiscovery.isLoaded = true;
+
+    await extensionDiscovery.initMain();
+
+    extensionDiscovery.events.on("add", (extension: InstalledExtension) => {
+      expect(extension).toEqual({
+        absolutePath: expect.any(String),
+        id: normalize("node_modules/my-extension/package.json"),
+        isBundled: false,
+        isEnabled: false,
+        manifest:  {
+          name: "my-extension",
+        },
+        manifestPath: normalize("node_modules/my-extension/package.json"),
+      });
+      done();
+    });
+
+    addHandler(join(extensionDiscovery.localFolderPath, "/my-extension/package.json"));
+  });
+
+  it("doesn't emit add for added file under extension", async done => {
+    globalThis.__non_webpack_require__.mockImplementationOnce(() => ({
+      name: "my-extension"
+    }));
+    let addHandler: (filePath: string) => void;
+
+    const mockWatchInstance: any = {
+      on: jest.fn((event: string, handler: typeof addHandler) => {
+        if (event === "add") {
+          addHandler = handler;
+        }
+
+        return mockWatchInstance;
+      })
+    };
+
+    mockedWatch.mockImplementationOnce(() =>
+      (mockWatchInstance) as any
+    );
+    const extensionDiscovery = new ExtensionDiscovery();
+
+    // Need to force isLoaded to be true so that the file watching is started
+    extensionDiscovery.isLoaded = true;
+
+    await extensionDiscovery.initMain();
+
+    const onAdd = jest.fn();
+
+    extensionDiscovery.events.on("add", onAdd);
+
+    addHandler(join(extensionDiscovery.localFolderPath, "/my-extension/node_modules/dep/package.json"));
+
+    setTimeout(() => {
+      expect(onAdd).not.toHaveBeenCalled();
+      done();
+    }, 10);
+  });
+});

src/extensions/extension-discovery.ts

@@ -1,4 +1,4 @@
-import chokidar from "chokidar";
+import { watch } from "chokidar";
 import { ipcRenderer } from "electron";
 import { EventEmitter } from "events";
 import fs from "fs-extra";
@@ -138,7 +138,7 @@ export class ExtensionDiscovery {
     await this.whenLoaded;
 
     // chokidar works better than fs.watch
-    chokidar.watch(this.localFolderPath, {
+    watch(this.localFolderPath, {
       // For adding and removing symlinks to work, the depth has to be 1.
       depth: 1,
       // Try to wait until the file has been completely copied.
@@ -156,9 +156,18 @@ export class ExtensionDiscovery {
   }
 
   handleWatchFileAdd =  async (filePath: string) => {
-    if (path.basename(filePath) === manifestFilename) {
+    // e.g. "foo/package.json"
+    const relativePath = path.relative(this.localFolderPath, filePath);
+
+    // Converts "foo/package.json" to ["foo", "package.json"], where length of 2 implies
+    // that the added file is in a folder under local folder path.
+    // This safeguards against a file watch being triggered under a sub-directory which is not an extension.
+    const isUnderLocalFolderPath = relativePath.split(path.sep).length === 2;
+
+    if (path.basename(filePath) === manifestFilename && isUnderLocalFolderPath) {
       try {
         const absPath = path.dirname(filePath);
+
         // this.loadExtensionFromPath updates this.packagesJson
         const extension = await this.loadExtensionFromPath(absPath);
 

src/jest.setup.ts

@@ -1,3 +1,6 @@
 import fetchMock from "jest-fetch-mock";
 // rewire global.fetch to call 'fetchMock'
 fetchMock.enableMocks();
+
+// Mock __non_webpack_require__ for tests
+globalThis.__non_webpack_require__ = jest.fn();