fix(cypress-tags.js): changed GLOB option handling to fix issues with…

… commas in glob patterns Changed how GLOB option is passed and handled to prevent current issues with passing it through -env caused by Cypress expecting a comma delimited string of variables. fix #429
badeball · Jun 3, 2021 · d4bb762 · d4bb762
1 parent 77dbacb
commit d4bb762
Show file tree

Hide file tree

Showing 4 changed files with 166 additions and 18 deletions.
diff --git a/README.md b/README.md
@@ -391,6 +391,22 @@ You can use a glob expression to select which feature files should be included.
 
 Example:
 
+```shell
+  ./node_modules/.bin/cypress-tags run -g 'cypress/integration/**/*.feature'
+```
+
+or 
+
+```shell
+  ./node_modules/.bin/cypress-tags run --glob 'cypress/integration/**/*.feature'
+```
+
+or
+
+<span style="color:orange">*DEPRECATED*</span>
+
+This will not work if your glob pattern contains commas since Cypress expects comma delimited string of env variables.
+
 ```shell
   ./node_modules/.bin/cypress-tags run -e GLOB='cypress/integration/**/*.feature'
 ```

diff --git a/cypress-tags.js b/cypress-tags.js
@@ -5,30 +5,20 @@ const glob = require("glob");
 const fs = require("fs");
 const { execFileSync } = require("child_process");
 
-const { shouldProceedCurrentStep } = require("./lib/tagsHelper");
+const {
+  stripCLIArguments,
+  parseArgsOrDefault,
+  getGlobArg,
+  shouldProceedCurrentStep,
+} = require("./lib/tagsHelper");
 
 const debug = (message, ...rest) =>
   process.env.DEBUG
     ? // eslint-disable-next-line no-console
       console.log(`DEBUG: ${message}`, rest.length ? rest : "")
     : null;
 
-function parseArgsOrDefault(argPrefix, defaultValue) {
-  const matchedArg = process.argv
-    .slice(2)
-    .find((arg) => arg.includes(`${argPrefix}=`));
-
-  // Cypress requires env vars to be passed as comma separated list
-  // otherwise it only accepts the last provided variable,
-  // the way we replace here accomodates for that.
-  const argValue = matchedArg
-    ? matchedArg.replace(new RegExp(`.*${argPrefix}=`), "").replace(/,.*/, "")
-    : "";
-
-  return argValue !== "" ? argValue : defaultValue;
-}
-
-const envGlob = parseArgsOrDefault("GLOB", false);
+const envGlob = getGlobArg();
 const envTags = parseArgsOrDefault("TAGS", "");
 
 let specGlob = envGlob || "cypress/integration/**/*.feature";
@@ -122,7 +112,11 @@ try {
   if (featuresToRun.length || envTags === "") {
     execFileSync(
       getCypressExecutable(),
-      [...process.argv.slice(2), "--spec", featuresToRun.join(",")],
+      [
+        ...stripCLIArguments(["-g", "--glob"]),
+        "--spec",
+        featuresToRun.join(","),
+      ],
       {
         stdio: [process.stdin, process.stdout, process.stderr],
       }

diff --git a/lib/tagsHelper.js b/lib/tagsHelper.js
@@ -1,3 +1,4 @@
+const minimist = require("minimist");
 const { TagExpressionParser } = require("cucumber-tag-expressions");
 
 function getEnvTags() {
@@ -17,7 +18,74 @@ function shouldProceedCurrentStep(tags = [], envTags = getEnvTags()) {
   }
 }
 
+/*
+  This is not a perfect solution since process.argv filtering should
+  probably be done using a parser to cover all edge cases.
+ */
+
+function stripCLIArguments(argsToRemove = []) {
+  const processArgv = process.argv.slice(2);
+
+  let prevValue = null;
+  for (let i = processArgv.length - 1; i >= 0; i -= 1) {
+    if (/^--?/.test(processArgv[i])) {
+      // eslint-disable-next-line no-loop-func
+      argsToRemove.forEach((arg) => {
+        const shortFlagChar = arg.slice(-1);
+        if (processArgv[i] === arg) {
+          processArgv.splice(i, prevValue ? 2 : 1);
+        } else if (new RegExp(`^${arg}=`, "g").test(processArgv[i])) {
+          processArgv.splice(i, 1);
+        } else if (
+          /^-[^-]+/.test(processArgv[i]) &&
+          /^-[^-]+/.test(arg) &&
+          processArgv[i].includes(shortFlagChar)
+        ) {
+          const flagPosition = processArgv[i].indexOf(shortFlagChar);
+          processArgv[i] = processArgv[i].replace(shortFlagChar, "");
+          if (prevValue && flagPosition === processArgv[i].length) {
+            processArgv.splice(i + 1, 1);
+          }
+        }
+      });
+      prevValue = null;
+    } else {
+      prevValue = processArgv[i];
+    }
+  }
+  return processArgv;
+}
+/**
+ * Users will be expected to pass args by --glob/-g to avoid issues related to commas in those parameters.
+ */
+function parseArgsOrDefault(argPrefix, defaultValue) {
+  const matchedArg = process.argv
+    .slice(2)
+    .find((arg) => arg.includes(`${argPrefix}=`));
+
+  // Cypress requires env vars to be passed as comma separated list
+  // otherwise it only accepts the last provided variable,
+  // the way we replace here accomodates for that.
+  const argValue = matchedArg
+    ? matchedArg.replace(new RegExp(`.*${argPrefix}=`), "").replace(/,.*/, "")
+    : "";
+
+  return argValue !== "" ? argValue : defaultValue;
+}
+
+function getGlobArg() {
+  const args = minimist(process.argv.slice(2), {
+    alias: { g: "glob" },
+    string: ["g"],
+  });
+
+  return args.g || parseArgsOrDefault("GLOB", false);
+}
+
 module.exports = {
   shouldProceedCurrentStep,
   getEnvTags,
+  stripCLIArguments,
+  parseArgsOrDefault,
+  getGlobArg,
 };
diff --git a/lib/tagsHelper.test.js b/lib/tagsHelper.test.js
@@ -0,0 +1,70 @@
+const { stripCLIArguments, getGlobArg } = require("./tagsHelper");
+
+const processArgv = process.argv;
+
+describe("stripCLIArguments function should return correct args array", () => {
+  beforeEach(() => {
+    process.argv = ["path/to/node", "path/to/file", "run"];
+  });
+  test("when only target option is present", () => {
+    process.argv.push(...["-g", "cypress/integration/**/*.feature"]);
+    expect(stripCLIArguments(["-g"])).to.eql(["run"]);
+  });
+  test("when target option is a word", () => {
+    process.argv.push(...["--glob", "cypress/integration/**/*.feature"]);
+    expect(stripCLIArguments(["--glob"])).to.eql(["run"]);
+  });
+  test("when option doesn't have a value tied to it", () => {
+    process.argv.push(...["-g"]);
+    expect(stripCLIArguments(["-g"])).to.eql(["run"]);
+  });
+  test("when there are multiple options but only one is target for removal", () => {
+    process.argv.push(...["-g", "-t"]);
+    expect(stripCLIArguments(["-g"])).to.eql(["run", "-t"]);
+  });
+  test("when there are multiple options to remove", () => {
+    process.argv.push(...["-g", "-t", "cypress tags string"]);
+    expect(stripCLIArguments(["-g", "-t"])).to.eql(["run"]);
+  });
+
+  test("when option is coupled with other ones like -tg", () => {
+    process.argv.push(...["-tg", "cypress/integration/**/*.feature"]);
+    expect(stripCLIArguments(["-t"])).to.eql([
+      "run",
+      "-g",
+      "cypress/integration/**/*.feature",
+    ]);
+  });
+
+  test("when option is coupled with other ones like -gt where t has a value", () => {
+    process.argv.push(...["-gt", "cypress tags string"]);
+    expect(stripCLIArguments(["-t"])).to.eql(["run", "-g"]);
+  });
+
+  afterEach(() => {
+    process.argv = processArgv;
+  });
+});
+
+describe("getGlobArg function should return", () => {
+  beforeEach(() => {
+    process.argv = ["path/to/node", "path/to/file", "run"];
+  });
+  test("glob pattern when using -g or --glob option", () => {
+    process.argv.push(...["-g", "cypress/integration/**/*.feature"]);
+    expect(getGlobArg()).to.equal("cypress/integration/**/*.feature");
+  });
+  test("glob pattern containing commas when using -g option", () => {
+    process.argv.push(...["-g", "cypress/integration/**/{1,2}*.feature"]);
+    expect(getGlobArg()).to.equal("cypress/integration/**/{1,2}*.feature");
+  });
+
+  test("glob pattern when using env variables GLOB=", () => {
+    process.argv.push(...["-e", "GLOB=cypress/integration/**/*.feature"]);
+    expect(getGlobArg()).to.equal("cypress/integration/**/*.feature");
+  });
+
+  afterEach(() => {
+    process.argv = processArgv;
+  });
+});