Standardise test and linting execution (#25)

package.json

@@ -6,8 +6,8 @@
     "bot-scripts": "./bin/index.js"
   },
   "scripts": {
-    "test": "jest --env node --setupFilesAfterEnv ./scripts/test/mock-logger.js",
-    "lint": "eslint . --config=./scripts/lint/eslintrc.json"
+    "test": "node ./scripts/test",
+    "lint": "node ./scripts/lint ."
   },
   "dependencies": {
     "botkit": "^0.7.0",

scripts/lint/index.js

@@ -1,29 +1,16 @@
-process.on("unhandledRejection", function(err) {
-  throw err;
-});
-
 const path = require("path");
-const spawn = require("cross-spawn");
-const { getBotDirectory, getArgs } = require("../utils");
+const {
+  getBotDirectory,
+  getArgs,
+  startsWithPath,
+  setupScript
+} = require("../shared");
 
+const execute = setupScript("eslint");
 const botDirectory = getBotDirectory();
 const args = getArgs();
 const lintingFiles = path.resolve(botDirectory, "./src");
 const eslintConfig = path.resolve(__dirname, "./eslintrc.json");
-const eslint = path.resolve(
-  require.resolve("eslint").split("eslint")[0],
-  "./eslint/bin/eslint.js"
-);
-
-const result = spawn.sync(
-  eslint,
-  [lintingFiles, "--config", eslintConfig].concat(args),
-  { stdio: "inherit" }
-);
 
-if (result.error) {
-  console.error(result.error); // eslint-disable-line no-console
-  process.exit(1);
-}
-if (result.signal) process.exit(1);
-process.exit(result.status);
+const cwd = startsWithPath(args) ? args.shift() : lintingFiles;
+execute([cwd, "--config", eslintConfig].concat(args));

scripts/lint/tests/index.spec.js

@@ -1,71 +1,91 @@
-global.process.on = jest.fn();
-global.process.exit = jest.fn();
-console.error = jest.fn(); // eslint-disable-line no-console
+const mockSetupScript = jest.fn();
+const mockGetArgs = jest.fn();
 
-let result;
-const mockSpawnSync = jest.fn(() => result);
-jest.mock("cross-spawn", () => ({ sync: mockSpawnSync }));
-jest.mock("../../utils", () => ({
-  getBotDirectory: () => "foo",
-  getArgs: () => ["--fix"]
-}));
+jest.mock("../../shared", () =>
+  Object.assign(require.requireActual("../../shared"), {
+    getBotDirectory: () => "foo",
+    getArgs: mockGetArgs,
+    setupScript: () => mockSetupScript
+  })
+);
 
 describe("lint script", function() {
   beforeEach(function() {
     jest.clearAllMocks();
     jest.resetModules();
-    result = {};
   });
 
-  describe("script success", function() {
+  describe("when no additional arguments", function() {
     beforeEach(function() {
-      result.status = 0;
+      mockGetArgs.mockReturnValue([]);
       require("../"); // eslint-disable-line global-require
     });
 
     it("calls eslint with the correct arguments", function() {
-      expect(mockSpawnSync.mock.calls.length).toBe(1);
-      expect(mockSpawnSync.mock.calls[0]).toEqual([
-        expect.stringContaining(
-          "/bot-scripts/node_modules/eslint/bin/eslint.js"
-        ),
+      expect(mockSetupScript.mock.calls.length).toBe(1);
+      expect(mockSetupScript.mock.calls[0]).toEqual([
         [
           expect.stringContaining("/bot-scripts/foo/src"),
           "--config",
-          expect.stringContaining("/bot-scripts/scripts/lint/eslintrc.json"),
-          "--fix"
-        ],
-        { stdio: "inherit" }
+          expect.stringContaining("/bot-scripts/scripts/lint/eslintrc.json")
+        ]
       ]);
     });
+  });
+
+  describe("when additional flag argument passed", function() {
+    beforeEach(function() {
+      mockGetArgs.mockReturnValue(["--fix"]);
+      require("../"); // eslint-disable-line global-require
+    });
 
-    it("exits with success", function() {
-      expect(process.exit.mock.calls.length).toBe(1);
-      expect(process.exit).toHaveBeenCalledWith(0);
+    it("calls eslint with the correct arguments", function() {
+      expect(mockSetupScript.mock.calls.length).toBe(1);
+      expect(mockSetupScript.mock.calls[0]).toEqual([
+        [
+          expect.stringContaining("/bot-scripts/foo/src"),
+          "--config",
+          expect.stringContaining("/bot-scripts/scripts/lint/eslintrc.json"),
+          "--fix"
+        ]
+      ]);
     });
   });
 
-  describe("script interupted", function() {
+  describe("when path argument passed", function() {
     beforeEach(function() {
-      result.signal = true;
+      mockGetArgs.mockReturnValue(["/new/lint/path"]);
       require("../"); // eslint-disable-line global-require
     });
 
-    it("exits with failure", function() {
-      expect(process.exit.mock.calls.length).toBe(2);
-      expect(process.exit).toHaveBeenCalledWith(1);
+    it("calls eslint with the correct arguments", function() {
+      expect(mockSetupScript.mock.calls.length).toBe(1);
+      expect(mockSetupScript.mock.calls[0]).toEqual([
+        [
+          "/new/lint/path",
+          "--config",
+          expect.stringContaining("/bot-scripts/scripts/lint/eslintrc.json")
+        ]
+      ]);
     });
   });
 
-  describe("script error", function() {
+  describe("when mixture of arguments passed", function() {
     beforeEach(function() {
-      result.error = true;
+      mockGetArgs.mockReturnValue(["/new/lint/path", "--fix"]);
       require("../"); // eslint-disable-line global-require
     });
 
-    it("exits with failure", function() {
-      expect(process.exit.mock.calls.length).toBe(2);
-      expect(process.exit).toHaveBeenCalledWith(1);
+    it("calls eslint with the correct arguments", function() {
+      expect(mockSetupScript.mock.calls.length).toBe(1);
+      expect(mockSetupScript.mock.calls[0]).toEqual([
+        [
+          "/new/lint/path",
+          "--config",
+          expect.stringContaining("/bot-scripts/scripts/lint/eslintrc.json"),
+          "--fix"
+        ]
+      ]);
     });
   });
 });

scripts/shared/__mocks__/test-tool.js

@@ -0,0 +1 @@
+module.exports = {};

scripts/shared/index.js

@@ -0,0 +1,35 @@
+const fs = require("fs");
+const path = require("path");
+const spawn = require("cross-spawn");
+
+const setupScript = function(tool) {
+  process.on("unhandledRejection", function(err) {
+    throw err;
+  });
+
+  const executable = path.resolve(
+    require.resolve(tool).split(tool)[0],
+    `./${tool}/bin/${tool}.js`
+  );
+
+  return function call(args) {
+    const result = spawn.sync(executable, args, { stdio: "inherit" });
+
+    if (result.error) {
+      console.error(result.error); // eslint-disable-line no-console
+      process.exit(1);
+    }
+    if (result.signal) process.exit(1);
+    process.exit(result.status);
+  };
+};
+
+module.exports = {
+  getArgs: () => process.argv.slice(2),
+  getBotDirectory: () => fs.realpathSync(process.cwd()),
+  startsWithPath: args =>
+    typeof args[0] === "string" &&
+    args[0].trim().length > 0 &&
+    !args[0].trim().startsWith("--"),
+  setupScript
+};

scripts/shared/tests/index.spec.js

@@ -0,0 +1,135 @@
+global.process.on = jest.fn();
+global.process.exit = jest.fn();
+console.error = jest.fn(); // eslint-disable-line no-console
+
+const mockSpawnSync = jest.fn();
+jest.mock("cross-spawn", () => ({ sync: mockSpawnSync }));
+jest.mock("test-tool");
+
+const {
+  getArgs,
+  getBotDirectory,
+  startsWithPath,
+  setupScript
+} = require("../");
+
+describe("shared task utils", function() {
+  let testContext;
+
+  beforeEach(function() {
+    jest.clearAllMocks();
+    jest.resetModules();
+    testContext = {};
+  });
+
+  describe("getArgs", function() {
+    beforeEach(function() {
+      global.process.argv = ["node-process", "file-path", "hello", "world"];
+    });
+
+    it("returns list of arguments", function() {
+      expect(getArgs()).toEqual(["hello", "world"]);
+    });
+  });
+
+  describe("getBotDirectory", function() {
+    it("returns bot directory", function() {
+      expect(getBotDirectory()).toEqual(
+        expect.stringMatching(/\/bot-scripts$/)
+      );
+    });
+  });
+
+  describe("startsWithPath", function() {
+    describe("empty arguments", function() {
+      beforeEach(function() {
+        testContext.args = [];
+      });
+
+      it("returns false", function() {
+        expect(startsWithPath(testContext.args)).toBe(false);
+      });
+    });
+
+    describe("flag arguments", function() {
+      beforeEach(function() {
+        testContext.args = ["--watch", "--fix"];
+      });
+
+      it("returns false", function() {
+        expect(startsWithPath(testContext.args)).toBe(false);
+      });
+    });
+
+    describe("arguments with leading path", function() {
+      beforeEach(function() {
+        testContext.args = ["/path/to/file", "--fix"];
+      });
+
+      it("returns true", function() {
+        expect(startsWithPath(testContext.args)).toBe(true);
+      });
+    });
+
+    describe("arguments which include path", function() {
+      beforeEach(function() {
+        testContext.args = ["--watch", "/path/to/file ", "--fix"];
+      });
+
+      it("returns true", function() {
+        expect(startsWithPath(testContext.args)).toBe(false);
+      });
+    });
+  });
+
+  describe("setupScript", function() {
+    beforeEach(function() {
+      testContext.scriptExecutor = setupScript("test-tool");
+    });
+
+    describe("script success", function() {
+      beforeEach(function() {
+        mockSpawnSync.mockReturnValue({ status: 0 });
+        testContext.scriptExecutor(["foo"]);
+      });
+
+      it("calls eslint with the correct arguments", function() {
+        expect(mockSpawnSync.mock.calls.length).toBe(1);
+        expect(mockSpawnSync.mock.calls[0]).toEqual([
+          expect.stringContaining("/test-tool/bin/test-tool.js"),
+          ["foo"],
+          { stdio: "inherit" }
+        ]);
+      });
+
+      it("exits with success", function() {
+        expect(process.exit.mock.calls.length).toBe(1);
+        expect(process.exit).toHaveBeenCalledWith(0);
+      });
+    });
+
+    describe("script interupted", function() {
+      beforeEach(function() {
+        mockSpawnSync.mockReturnValue({ signal: true });
+        testContext.scriptExecutor(["foo"]);
+      });
+
+      it("exits with failure", function() {
+        expect(process.exit.mock.calls.length).toBe(2);
+        expect(process.exit).toHaveBeenCalledWith(1);
+      });
+    });
+
+    describe("script error", function() {
+      beforeEach(function() {
+        mockSpawnSync.mockReturnValue({ error: true });
+        testContext.scriptExecutor(["foo"]);
+      });
+
+      it("exits with failure", function() {
+        expect(process.exit.mock.calls.length).toBe(2);
+        expect(process.exit).toHaveBeenCalledWith(1);
+      });
+    });
+  });
+});

scripts/start/index.js

@@ -3,7 +3,7 @@ process.on("unhandledRejection", function(err) {
 });
 
 const path = require("path");
-const { getBotDirectory } = require("../utils");
+const { getBotDirectory } = require("../shared");
 const botDirectory = getBotDirectory();
 
 const botPath = path.resolve(botDirectory, "./src/index.js");

scripts/test/index.js

@@ -1,20 +1,17 @@
-process.on("unhandledRejection", function(err) {
-  throw err;
-});
-
 const path = require("path");
-const jest = require("jest");
-const { getBotDirectory, getArgs } = require("../utils");
+const {
+  getBotDirectory,
+  getArgs,
+  startsWithPath,
+  setupScript
+} = require("../shared");
 
+const execute = setupScript("jest");
 const botDirectory = getBotDirectory();
 const args = getArgs();
 const mockLogger = path.resolve(__dirname, "./mock-logger.js");
-jest.run(
-  [botDirectory].concat(
-    args,
-    "--env",
-    "node",
-    "--setupFilesAfterEnv",
-    mockLogger
-  )
+
+const cwd = startsWithPath(args) ? args.shift() : botDirectory;
+execute(
+  [cwd, "--env", "node", "--setupFilesAfterEnv", mockLogger].concat(args)
 );