From 74c13fdc953c6846782863810ec08ab8d310ea80 Mon Sep 17 00:00:00 2001
From: Dinika Saxena <dinikasaxenas@gmail.com>
Date: Mon, 25 Nov 2024 14:20:18 +0100
Subject: [PATCH 01/13] Fix 5028 // Numerical arguments to cli throw uncaught
 error

This PR throws a custom error which is (hopefully) easier to understand
when:

i. a numerical argument is passed to mocha cli
ii. numerical value is used as a value for one of the mocha flags that is not compatible with
numerical values.

Signed-off-by: Dinika Saxena <dinika@greyllama.cc>
---
 lib/cli/options.js                 | 23 ++++++++------
 test/node-unit/cli/options.spec.js | 49 +++++++++++++++++++++++++++---
 2 files changed, 58 insertions(+), 14 deletions(-)

diff --git a/lib/cli/options.js b/lib/cli/options.js
index fc0c951a8c..7886b510a8 100644
--- a/lib/cli/options.js
+++ b/lib/cli/options.js
@@ -110,15 +110,19 @@ const parse = (args = [], defaultValues = {}, ...configObjects) => {
   // 4. we can then reapply the values after yargs-parser is done.
   const nodeArgs = (Array.isArray(args) ? args : args.split(' ')).reduce(
     (acc, arg) => {
-      const pair = arg.split('=');
-      let flag = pair[0];
-      if (isNodeFlag(flag, false)) {
-        flag = flag.replace(/^--?/, '');
-        return arg.includes('=')
-          ? acc.concat([[flag, pair[1]]])
-          : acc.concat([[flag, true]]);
+      try {
+        const pair = arg.split('=');
+        let flag = pair[0];
+        if (isNodeFlag(flag, false)) {
+          flag = flag.replace(/^--?/, '');
+          return arg.includes('=')
+            ? acc.concat([[flag, pair[1]]])
+            : acc.concat([[flag, true]]);
+        }
+        return acc;
+      } catch (err) {
+        throw new Error(`Invalid option ${arg} passed to mocha cli`);
       }
-      return acc;
     },
     []
   );
@@ -192,8 +196,7 @@ const loadPkgRc = (args = {}) => {
           filepath
         );
       } else {
-        debug('failed to read default package.json at %s; ignoring',
-              filepath);
+        debug('failed to read default package.json at %s; ignoring', filepath);
         return result;
       }
     }
diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js
index 7c846a37ed..22ed6830bb 100644
--- a/test/node-unit/cli/options.spec.js
+++ b/test/node-unit/cli/options.spec.js
@@ -204,9 +204,7 @@ describe('options', function () {
             const filepath = '/some/package.json';
             readFileSync = sinon.stub();
             // package.json
-            readFileSync
-              .onFirstCall()
-              .returns('{definitely-invalid');
+            readFileSync.onFirstCall().returns('{definitely-invalid');
             findConfig = sinon.stub().returns('/some/.mocharc.json');
             loadConfig = sinon.stub().returns({});
             findupSync = sinon.stub().returns(filepath);
@@ -224,7 +222,7 @@ describe('options', function () {
                 loadOptions();
               },
               'to throw',
-                /SyntaxError/,
+              /SyntaxError/
             );
           });
         });
@@ -676,5 +674,48 @@ describe('options', function () {
         ]);
       });
     });
+
+    describe('"numerical arguments"', function () {
+      const numericalArg = 123;
+
+      beforeEach(function () {
+        readFileSync = sinon.stub();
+        findConfig = sinon.stub();
+        loadConfig = sinon.stub();
+        findupSync = sinon.stub();
+        loadOptions = proxyLoadOptions({
+          readFileSync,
+          findConfig,
+          loadConfig,
+          findupSync
+        });
+      });
+
+      it('throws error when numerical option is passed to cli', function () {
+        expect(() => loadOptions(`${numericalArg}`), 'to throw', {
+          message: `Invalid option ${numericalArg} passed to mocha cli`
+        });
+      });
+
+      it('throws error when numerical argument is passed to mocha flag that does not accept numerical value', function () {
+        expect(() => loadOptions(`--delay ${numericalArg}`), 'to throw', {
+          message: `Invalid option ${numericalArg} passed to mocha cli`
+        });
+      });
+
+      it('does not throw error if numerical value is passed to a compatible mocha flag', function () {
+        expect(() => loadOptions(`--retries ${numericalArg}`), 'not to throw');
+      });
+
+      it('does not throw error if numerical value is passed to a node options', function () {
+        expect(
+          () =>
+            loadOptions(
+              `--secure-heap-min=${numericalArg} --conditions=${numericalArg}`
+            ),
+          'not to throw'
+        );
+      });
+    });
   });
 });

From 26dd073c17c25488df2d10de9f19c7005c99d388 Mon Sep 17 00:00:00 2001
From: Dinika Saxena <dinikasaxenas@gmail.com>
Date: Thu, 28 Nov 2024 19:28:05 +0100
Subject: [PATCH 02/13] Use type-check to throw error instead of a broad
 try/catch

Signed-off-by: Dinika Saxena <dinikasaxenas@gmail.com>
---
 lib/cli/options.js                 | 24 ++++++++++++------------
 test/node-unit/cli/options.spec.js |  6 ++++--
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/lib/cli/options.js b/lib/cli/options.js
index 7886b510a8..b7779025b2 100644
--- a/lib/cli/options.js
+++ b/lib/cli/options.js
@@ -110,19 +110,18 @@ const parse = (args = [], defaultValues = {}, ...configObjects) => {
   // 4. we can then reapply the values after yargs-parser is done.
   const nodeArgs = (Array.isArray(args) ? args : args.split(' ')).reduce(
     (acc, arg) => {
-      try {
-        const pair = arg.split('=');
-        let flag = pair[0];
-        if (isNodeFlag(flag, false)) {
-          flag = flag.replace(/^--?/, '');
-          return arg.includes('=')
-            ? acc.concat([[flag, pair[1]]])
-            : acc.concat([[flag, true]]);
-        }
-        return acc;
-      } catch (err) {
+      if (typeof arg !== 'string') {
         throw new Error(`Invalid option ${arg} passed to mocha cli`);
       }
+      const pair = arg.split('=');
+      let flag = pair[0];
+      if (isNodeFlag(flag, false)) {
+        flag = flag.replace(/^--?/, '');
+        return arg.includes('=')
+          ? acc.concat([[flag, pair[1]]])
+          : acc.concat([[flag, true]]);
+      }
+      return acc;
     },
     []
   );
@@ -196,7 +195,8 @@ const loadPkgRc = (args = {}) => {
           filepath
         );
       } else {
-        debug('failed to read default package.json at %s; ignoring', filepath);
+        debug('failed to read default package.json at %s; ignoring',
+              filepath);
         return result;
       }
     }
diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js
index 22ed6830bb..69357449eb 100644
--- a/test/node-unit/cli/options.spec.js
+++ b/test/node-unit/cli/options.spec.js
@@ -204,7 +204,9 @@ describe('options', function () {
             const filepath = '/some/package.json';
             readFileSync = sinon.stub();
             // package.json
-            readFileSync.onFirstCall().returns('{definitely-invalid');
+            readFileSync
+              .onFirstCall()
+              .returns('{definitely-invalid');
             findConfig = sinon.stub().returns('/some/.mocharc.json');
             loadConfig = sinon.stub().returns({});
             findupSync = sinon.stub().returns(filepath);
@@ -222,7 +224,7 @@ describe('options', function () {
                 loadOptions();
               },
               'to throw',
-              /SyntaxError/
+                /SyntaxError/,
             );
           });
         });

From 6c3387725427c28030f94fe6d694a5f15ad46c56 Mon Sep 17 00:00:00 2001
From: Dinika Saxena <dinikasaxenas@gmail.com>
Date: Mon, 2 Dec 2024 11:16:12 +0100
Subject: [PATCH 03/13] Rename numerical to numeric

Signed-off-by: Dinika Saxena <dinikasaxenas@gmail.com>
---
 test/node-unit/cli/options.spec.js | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js
index 69357449eb..7f38002fa7 100644
--- a/test/node-unit/cli/options.spec.js
+++ b/test/node-unit/cli/options.spec.js
@@ -204,9 +204,7 @@ describe('options', function () {
             const filepath = '/some/package.json';
             readFileSync = sinon.stub();
             // package.json
-            readFileSync
-              .onFirstCall()
-              .returns('{definitely-invalid');
+            readFileSync.onFirstCall().returns('{definitely-invalid');
             findConfig = sinon.stub().returns('/some/.mocharc.json');
             loadConfig = sinon.stub().returns({});
             findupSync = sinon.stub().returns(filepath);
@@ -224,7 +222,7 @@ describe('options', function () {
                 loadOptions();
               },
               'to throw',
-                /SyntaxError/,
+              /SyntaxError/
             );
           });
         });
@@ -677,8 +675,8 @@ describe('options', function () {
       });
     });
 
-    describe('"numerical arguments"', function () {
-      const numericalArg = 123;
+    describe('"numeric arguments"', function () {
+      const numericArg = 123;
 
       beforeEach(function () {
         readFileSync = sinon.stub();
@@ -693,27 +691,27 @@ describe('options', function () {
         });
       });
 
-      it('throws error when numerical option is passed to cli', function () {
-        expect(() => loadOptions(`${numericalArg}`), 'to throw', {
-          message: `Invalid option ${numericalArg} passed to mocha cli`
+      it('throws error when numeric option is passed to cli', function () {
+        expect(() => loadOptions(`${numericArg}`), 'to throw', {
+          message: `Invalid option ${numericArg} passed to mocha cli`
         });
       });
 
-      it('throws error when numerical argument is passed to mocha flag that does not accept numerical value', function () {
-        expect(() => loadOptions(`--delay ${numericalArg}`), 'to throw', {
-          message: `Invalid option ${numericalArg} passed to mocha cli`
+      it('throws error when numeric argument is passed to mocha flag that does not accept numeric value', function () {
+        expect(() => loadOptions(`--delay ${numericArg}`), 'to throw', {
+          message: `Invalid option ${numericArg} passed to mocha cli`
         });
       });
 
-      it('does not throw error if numerical value is passed to a compatible mocha flag', function () {
-        expect(() => loadOptions(`--retries ${numericalArg}`), 'not to throw');
+      it('does not throw error if numeric value is passed to a compatible mocha flag', function () {
+        expect(() => loadOptions(`--retries ${numericArg}`), 'not to throw');
       });
 
-      it('does not throw error if numerical value is passed to a node options', function () {
+      it('does not throw error if numeric value is passed to a node options', function () {
         expect(
           () =>
             loadOptions(
-              `--secure-heap-min=${numericalArg} --conditions=${numericalArg}`
+              `--secure-heap-min=${numericArg} --conditions=${numericArg}`
             ),
           'not to throw'
         );

From a8ea4dd92cd187656f35ef3f6e3670a7c4cd3430 Mon Sep 17 00:00:00 2001
From: Dinika Saxena <dinikasaxenas@gmail.com>
Date: Mon, 2 Dec 2024 19:15:30 +0100
Subject: [PATCH 04/13] Find flag for mocha cli after parsing yargs

---
 lib/cli/options.js             | 73 ++++++++++++++++++++++++----------
 lib/cli/run-option-metadata.js | 20 ++++++++++
 lib/utils.js                   |  7 ++++
 3 files changed, 79 insertions(+), 21 deletions(-)

diff --git a/lib/cli/options.js b/lib/cli/options.js
index b7779025b2..b25fe64db8 100644
--- a/lib/cli/options.js
+++ b/lib/cli/options.js
@@ -10,7 +10,12 @@
 const fs = require('fs');
 const ansi = require('ansi-colors');
 const yargsParser = require('yargs-parser');
-const {types, aliases} = require('./run-option-metadata');
+const {
+  types,
+  aliases,
+  isMochaFlag,
+  expectedTypeForFlag
+} = require('./run-option-metadata');
 const {ONE_AND_DONE_ARGS} = require('./one-and-dones');
 const mocharc = require('../mocharc.json');
 const {list} = require('./run-helpers');
@@ -18,7 +23,12 @@ const {loadConfig, findConfig} = require('./config');
 const findUp = require('find-up');
 const debug = require('debug')('mocha:cli:options');
 const {isNodeFlag} = require('./node-flags');
-const {createUnparsableFileError} = require('../errors');
+const {
+  createUnparsableFileError,
+  createInvalidArgumentTypeError,
+  createUnsupportedError
+} = require('../errors');
+const {isNumeric} = require('../utils');
 
 /**
  * The `yargs-parser` namespace
@@ -108,23 +118,21 @@ const parse = (args = [], defaultValues = {}, ...configObjects) => {
   // 3. to avoid explicitly defining the set of them, we tell yargs-parser they
   //    are ALL boolean flags.
   // 4. we can then reapply the values after yargs-parser is done.
-  const nodeArgs = (Array.isArray(args) ? args : args.split(' ')).reduce(
-    (acc, arg) => {
-      if (typeof arg !== 'string') {
-        throw new Error(`Invalid option ${arg} passed to mocha cli`);
-      }
-      const pair = arg.split('=');
-      let flag = pair[0];
-      if (isNodeFlag(flag, false)) {
-        flag = flag.replace(/^--?/, '');
-        return arg.includes('=')
-          ? acc.concat([[flag, pair[1]]])
-          : acc.concat([[flag, true]]);
-      }
-      return acc;
-    },
-    []
-  );
+  const allArgs = Array.isArray(args) ? args : args.split(' ');
+  const nodeArgs = allArgs.reduce((acc, arg) => {
+    if (typeof arg !== 'string') {
+      throw new Error(`Invalid option ${arg} passed to mocha cli`);
+    }
+    const pair = arg.split('=');
+    let flag = pair[0];
+    if (isNodeFlag(flag, false)) {
+      flag = flag.replace(/^--?/, '');
+      return arg.includes('=')
+        ? acc.concat([[flag, pair[1]]])
+        : acc.concat([[flag, true]]);
+    }
+    return acc;
+  }, []);
 
   const result = yargsParser.detailed(args, {
     configuration,
@@ -148,6 +156,30 @@ const parse = (args = [], defaultValues = {}, ...configObjects) => {
     result.argv[key] = value;
   });
 
+  const numericPositionalArgs = result.argv._.filter(arg => isNumeric(arg));
+  numericPositionalArgs.forEach(numericArg => {
+    const flag = allArgs
+      .map(arg => arg.replace(/^--?/, ''))
+      .find((arg, index) => {
+        return (
+          isMochaFlag(arg) &&
+          args[index + 1] === String(numericArg) &&
+          String(result.argv[arg]) !== String(numericArg)
+        );
+      });
+    if (flag) {
+      throw createInvalidArgumentTypeError(
+        `Flag ${flag} has invalid arg ${numericArg}`,
+        numericArg,
+        expectedTypeForFlag(flag)
+      );
+    } else {
+      throw createUnsupportedError(
+        `Invalid option ${numericArg} passed to mocha cli`
+      );
+    }
+  });
+
   return result.argv;
 };
 
@@ -195,8 +227,7 @@ const loadPkgRc = (args = {}) => {
           filepath
         );
       } else {
-        debug('failed to read default package.json at %s; ignoring',
-              filepath);
+        debug('failed to read default package.json at %s; ignoring', filepath);
         return result;
       }
     }
diff --git a/lib/cli/run-option-metadata.js b/lib/cli/run-option-metadata.js
index 83aa70dd7a..44ebf1e988 100644
--- a/lib/cli/run-option-metadata.js
+++ b/lib/cli/run-option-metadata.js
@@ -114,3 +114,23 @@ const ALL_MOCHA_FLAGS = Object.keys(TYPES).reduce((acc, key) => {
 exports.isMochaFlag = flag => {
   return ALL_MOCHA_FLAGS.has(flag.replace(/^--?/, ''));
 };
+
+/**
+ * Returns expected yarg option type for a given mocha flag.
+ * @param {string} flag - Flag to check (can be with out without leading "--"")
+ * @returns {string | undefined} - If flag is a valid mocha flag, the expected type of argument for this flag is returned, otherwise undefined is returned.
+ */
+exports.expectedTypeForFlag = flag => {
+  const normalizedName = flag?.replace(/^--?/, '');
+
+  // If flag is an alias, get it's full name.
+  const aliases = exports.aliases;
+  const fullFlagName =
+    Object.keys(aliases).find(flagName =>
+      aliases[flagName].includes(normalizedName)
+    ) || normalizedName;
+
+  return Object.keys(TYPES).find(flagType =>
+    TYPES[flagType].includes(fullFlagName)
+  );
+};
diff --git a/lib/utils.js b/lib/utils.js
index 31b313a6e0..89b21a32d6 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -689,3 +689,10 @@ exports.breakCircularDeps = inputObj => {
 
   return _breakCircularDeps(inputObj);
 };
+
+/**
+ * Checks if provided input can be parsed as a JavaScript Number.
+ */
+exports.isNumeric = input => {
+  return !isNaN(parseFloat(input));
+};

From 4e8ffce181a4d01beb4bf4b47b069a568fb9d474 Mon Sep 17 00:00:00 2001
From: Dinika Saxena <dinikasaxenas@gmail.com>
Date: Tue, 3 Dec 2024 10:03:11 +0100
Subject: [PATCH 05/13] Find flag for faulty numeric args before yargs parser

Signed-off-by: Dinika Saxena <dinikasaxenas@gmail.com>
---
 lib/cli/options.js                     | 51 ++++++++---------
 lib/cli/run-option-metadata.js         | 13 +----
 test/node-unit/cli/mocha-flags.spec.js | 27 +++++++++
 test/node-unit/cli/options.spec.js     | 78 ++++++++++++++++++++++----
 test/node-unit/utils.spec.js           | 11 ++++
 5 files changed, 128 insertions(+), 52 deletions(-)
 create mode 100644 test/node-unit/cli/mocha-flags.spec.js

diff --git a/lib/cli/options.js b/lib/cli/options.js
index b25fe64db8..9768206bcc 100644
--- a/lib/cli/options.js
+++ b/lib/cli/options.js
@@ -114,15 +114,31 @@ const nargOpts = types.array
 const parse = (args = [], defaultValues = {}, ...configObjects) => {
   // save node-specific args for special handling.
   // 1. when these args have a "=" they should be considered to have values
-  // 2. if they don't, they just boolean flags
+  // 2. if they don't, they are just boolean flags
   // 3. to avoid explicitly defining the set of them, we tell yargs-parser they
   //    are ALL boolean flags.
   // 4. we can then reapply the values after yargs-parser is done.
   const allArgs = Array.isArray(args) ? args : args.split(' ');
-  const nodeArgs = allArgs.reduce((acc, arg) => {
-    if (typeof arg !== 'string') {
-      throw new Error(`Invalid option ${arg} passed to mocha cli`);
+  const nodeArgs = allArgs.reduce((acc, arg, index, allArgs) => {
+    const maybeFlag = allArgs[index - 1];
+
+    if (isNumeric(arg) && (!maybeFlag || !isMochaFlag(maybeFlag))) {
+      throw createUnsupportedError(
+        `Option ${arg} is unsupported by the mocha cli`
+      );
+    }
+    if (
+      isNumeric(arg) &&
+      isMochaFlag(maybeFlag) &&
+      expectedTypeForFlag(maybeFlag) !== 'number'
+    ) {
+      throw createInvalidArgumentTypeError(
+        `Mocha flag '--${maybeFlag}' given invalid option: '${arg}'`,
+        Number(arg),
+        expectedTypeForFlag(maybeFlag)
+      );
     }
+
     const pair = arg.split('=');
     let flag = pair[0];
     if (isNodeFlag(flag, false)) {
@@ -156,30 +172,6 @@ const parse = (args = [], defaultValues = {}, ...configObjects) => {
     result.argv[key] = value;
   });
 
-  const numericPositionalArgs = result.argv._.filter(arg => isNumeric(arg));
-  numericPositionalArgs.forEach(numericArg => {
-    const flag = allArgs
-      .map(arg => arg.replace(/^--?/, ''))
-      .find((arg, index) => {
-        return (
-          isMochaFlag(arg) &&
-          args[index + 1] === String(numericArg) &&
-          String(result.argv[arg]) !== String(numericArg)
-        );
-      });
-    if (flag) {
-      throw createInvalidArgumentTypeError(
-        `Flag ${flag} has invalid arg ${numericArg}`,
-        numericArg,
-        expectedTypeForFlag(flag)
-      );
-    } else {
-      throw createUnsupportedError(
-        `Invalid option ${numericArg} passed to mocha cli`
-      );
-    }
-  });
-
   return result.argv;
 };
 
@@ -227,7 +219,8 @@ const loadPkgRc = (args = {}) => {
           filepath
         );
       } else {
-        debug('failed to read default package.json at %s; ignoring', filepath);
+        debug('failed to read default package.json at %s; ignoring',
+              filepath);
         return result;
       }
     }
diff --git a/lib/cli/run-option-metadata.js b/lib/cli/run-option-metadata.js
index 44ebf1e988..7deda70784 100644
--- a/lib/cli/run-option-metadata.js
+++ b/lib/cli/run-option-metadata.js
@@ -50,17 +50,8 @@ const TYPES = (exports.types = {
     'sort',
     'watch'
   ],
-  number: ['retries', 'jobs'],
-  string: [
-    'config',
-    'fgrep',
-    'grep',
-    'package',
-    'reporter',
-    'ui',
-    'slow',
-    'timeout'
-  ]
+  number: ['retries', 'jobs', 'slow', 'timeout'],
+  string: ['config', 'fgrep', 'grep', 'package', 'reporter', 'ui']
 });
 
 /**
diff --git a/test/node-unit/cli/mocha-flags.spec.js b/test/node-unit/cli/mocha-flags.spec.js
new file mode 100644
index 0000000000..dbd56334b2
--- /dev/null
+++ b/test/node-unit/cli/mocha-flags.spec.js
@@ -0,0 +1,27 @@
+'use strict';
+
+const {
+  types,
+  expectedTypeForFlag
+} = require('../../../lib/cli/run-option-metadata');
+
+describe('mocha-flags', function () {
+  describe('expectedTypeForFlag()', function () {
+    it('returns expected type for all mocha flags', function () {
+      Object.entries(types).forEach(([dataType, flags]) => {
+        flags.forEach(flag => {
+          expect(expectedTypeForFlag(flag), 'to equal', dataType);
+        });
+      });
+    });
+
+    it('returns undefined for node flags', function () {
+      expect(expectedTypeForFlag('--throw-deprecation'), 'to equal', undefined);
+      expect(expectedTypeForFlag('throw-deprecation'), 'to equal', undefined);
+    });
+
+    it('returns undefined for unsupported flags', function () {
+      expect(expectedTypeForFlag('--foo'), 'to equal', undefined);
+    });
+  });
+});
diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js
index 7f38002fa7..4e9e381c64 100644
--- a/test/node-unit/cli/options.spec.js
+++ b/test/node-unit/cli/options.spec.js
@@ -3,6 +3,7 @@
 const sinon = require('sinon');
 const rewiremock = require('rewiremock/node');
 const {ONE_AND_DONE_ARGS} = require('../../../lib/cli/one-and-dones');
+const {constants} = require('../../../lib/errors');
 
 const modulePath = require.resolve('../../../lib/cli/options');
 const mocharcPath = require.resolve('../../../lib/mocharc.json');
@@ -204,7 +205,9 @@ describe('options', function () {
             const filepath = '/some/package.json';
             readFileSync = sinon.stub();
             // package.json
-            readFileSync.onFirstCall().returns('{definitely-invalid');
+            readFileSync
+              .onFirstCall()
+              .returns('{definitely-invalid');
             findConfig = sinon.stub().returns('/some/.mocharc.json');
             loadConfig = sinon.stub().returns({});
             findupSync = sinon.stub().returns(filepath);
@@ -222,7 +225,7 @@ describe('options', function () {
                 loadOptions();
               },
               'to throw',
-              /SyntaxError/
+                /SyntaxError/,
             );
           });
         });
@@ -292,7 +295,6 @@ describe('options', function () {
 
             result = loadOptions('--no-diff --no-config');
           });
-
           it('should return parsed args, default config and package.json', function () {
             expect(
               result,
@@ -524,7 +526,7 @@ describe('options', function () {
           loadOptions('--timeout 500'),
           'to have property',
           'timeout',
-          '500'
+          500
         );
       });
 
@@ -678,6 +680,23 @@ describe('options', function () {
     describe('"numeric arguments"', function () {
       const numericArg = 123;
 
+      const unsupportedError = arg => {
+        return {
+          message: `Option ${arg} is unsupported by the mocha cli`,
+          code: constants.UNSUPPORTED
+        };
+      };
+
+      const invalidArgError = (flag, arg, expectedType = 'string') => {
+        return {
+          message: `Mocha flag '--${flag}' given invalid option: '${arg}'`,
+          code: constants.INVALID_ARG_TYPE,
+          argument: arg,
+          actual: 'number',
+          expected: expectedType
+        };
+      };
+
       beforeEach(function () {
         readFileSync = sinon.stub();
         findConfig = sinon.stub();
@@ -691,16 +710,51 @@ describe('options', function () {
         });
       });
 
-      it('throws error when numeric option is passed to cli', function () {
-        expect(() => loadOptions(`${numericArg}`), 'to throw', {
-          message: `Invalid option ${numericArg} passed to mocha cli`
-        });
+      it('throws UNSUPPORTED error when numeric option is passed to cli', function () {
+        expect(
+          () => loadOptions(`${numericArg}`),
+          'to throw',
+          unsupportedError(numericArg)
+        );
       });
 
-      it('throws error when numeric argument is passed to mocha flag that does not accept numeric value', function () {
-        expect(() => loadOptions(`--delay ${numericArg}`), 'to throw', {
-          message: `Invalid option ${numericArg} passed to mocha cli`
-        });
+      it('throws INVALID_ARG_TYPE error when numeric argument is passed to mocha flag that does not accept numeric value', function () {
+        const flag = '--delay';
+        expect(
+          () => loadOptions(`${flag} ${numericArg}`),
+          'to throw',
+          invalidArgError(flag, numericArg, 'boolean')
+        );
+      });
+
+      it('throws INVALID_ARG_TYPE error when incompatible flag does not have preceding "--"', function () {
+        const flag = 'delay';
+        expect(
+          () => loadOptions(`${flag} ${numericArg}`),
+          'to throw',
+          invalidArgError(flag, numericArg, 'boolean')
+        );
+      });
+
+      it('shows correct flag in error when multiple mocha flags have numeric values', function () {
+        const flag = '--delay';
+        expect(
+          () =>
+            loadOptions(
+              `--timeout ${numericArg} ${flag} ${numericArg} --retries ${numericArg}`
+            ),
+          'to throw',
+          invalidArgError(flag, numericArg, 'boolean')
+        );
+      });
+
+      it('throws UNSUPPORTED error when numeric arg is passed to unsupported flag', function () {
+        const invalidFlag = 'foo';
+        expect(
+          () => loadOptions(`${invalidFlag} ${numericArg}`),
+          'to throw',
+          unsupportedError(numericArg)
+        );
       });
 
       it('does not throw error if numeric value is passed to a compatible mocha flag', function () {
diff --git a/test/node-unit/utils.spec.js b/test/node-unit/utils.spec.js
index 1381244a33..d910de496c 100644
--- a/test/node-unit/utils.spec.js
+++ b/test/node-unit/utils.spec.js
@@ -45,5 +45,16 @@ describe('utils', function () {
         );
       });
     });
+    describe('isNumeric()', function () {
+      it('returns true for a number type', function () {
+        expect(utils.isNumeric(42), 'to equal', true);
+      });
+      it('returns true for a string that can be parsed as a number', function () {
+        expect(utils.isNumeric('42'), 'to equal', true);
+      });
+      it('returns false for a string that cannot be parsed as a number', function () {
+        expect(utils.isNumeric('foo'), 'to equal', false);
+      });
+    });
   });
 });

From 662a04e336a2bbcfe5276c031ab564f3b5064306 Mon Sep 17 00:00:00 2001
From: Dinika Saxena <dinikasaxenas@gmail.com>
Date: Tue, 3 Dec 2024 14:50:45 +0100
Subject: [PATCH 06/13] Add js-doc private keyword to fix netlify doc
 deployment

---
 lib/cli/run-option-metadata.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/cli/run-option-metadata.js b/lib/cli/run-option-metadata.js
index 7deda70784..0ce34c986c 100644
--- a/lib/cli/run-option-metadata.js
+++ b/lib/cli/run-option-metadata.js
@@ -110,6 +110,7 @@ exports.isMochaFlag = flag => {
  * Returns expected yarg option type for a given mocha flag.
  * @param {string} flag - Flag to check (can be with out without leading "--"")
  * @returns {string | undefined} - If flag is a valid mocha flag, the expected type of argument for this flag is returned, otherwise undefined is returned.
+ * @private
  */
 exports.expectedTypeForFlag = flag => {
   const normalizedName = flag?.replace(/^--?/, '');

From 09574d774135adc58b46bb9f58dadd750147cbd4 Mon Sep 17 00:00:00 2001
From: Dinika Saxena <dinikasaxenas@gmail.com>
Date: Wed, 4 Dec 2024 11:48:11 +0100
Subject: [PATCH 07/13] [Style] // Reduce code duplication

---
 lib/cli/options.js | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/cli/options.js b/lib/cli/options.js
index 9768206bcc..ed350ce82f 100644
--- a/lib/cli/options.js
+++ b/lib/cli/options.js
@@ -143,9 +143,7 @@ const parse = (args = [], defaultValues = {}, ...configObjects) => {
     let flag = pair[0];
     if (isNodeFlag(flag, false)) {
       flag = flag.replace(/^--?/, '');
-      return arg.includes('=')
-        ? acc.concat([[flag, pair[1]]])
-        : acc.concat([[flag, true]]);
+      return acc.concat([[flag, arg.includes('=') ? pair[1] : true]]);
     }
     return acc;
   }, []);

From 1deca6b16245ce4dc28bdbcc66bc207e7a09143a Mon Sep 17 00:00:00 2001
From: Dinika Saxena <dinikasaxenas@gmail.com>
Date: Wed, 4 Dec 2024 11:55:18 +0100
Subject: [PATCH 08/13] Add an "it" block for each flag for easier
 debug-ability

Signed-off-by: Dinika Saxena <dinikasaxenas@gmail.com>
---
 test/node-unit/cli/mocha-flags.spec.js | 6 +++---
 test/node-unit/cli/options.spec.js     | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/test/node-unit/cli/mocha-flags.spec.js b/test/node-unit/cli/mocha-flags.spec.js
index dbd56334b2..29647dea05 100644
--- a/test/node-unit/cli/mocha-flags.spec.js
+++ b/test/node-unit/cli/mocha-flags.spec.js
@@ -7,9 +7,9 @@ const {
 
 describe('mocha-flags', function () {
   describe('expectedTypeForFlag()', function () {
-    it('returns expected type for all mocha flags', function () {
-      Object.entries(types).forEach(([dataType, flags]) => {
-        flags.forEach(flag => {
+    Object.entries(types).forEach(([dataType, flags]) => {
+      flags.forEach(flag => {
+        it(`returns expected ${flag}'s type as ${dataType}`, function () {
           expect(expectedTypeForFlag(flag), 'to equal', dataType);
         });
       });
diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js
index 4e9e381c64..a99812d328 100644
--- a/test/node-unit/cli/options.spec.js
+++ b/test/node-unit/cli/options.spec.js
@@ -295,6 +295,7 @@ describe('options', function () {
 
             result = loadOptions('--no-diff --no-config');
           });
+
           it('should return parsed args, default config and package.json', function () {
             expect(
               result,

From 8f727b38f41ab25dd453041adaf49f5162b6e0ee Mon Sep 17 00:00:00 2001
From: Dinika Saxena <dinikasaxenas@gmail.com>
Date: Wed, 4 Dec 2024 17:26:41 +0100
Subject: [PATCH 09/13] Remove ? from flag since it cannot be undefined

---
 lib/cli/run-option-metadata.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/cli/run-option-metadata.js b/lib/cli/run-option-metadata.js
index 0ce34c986c..0aabd2ad93 100644
--- a/lib/cli/run-option-metadata.js
+++ b/lib/cli/run-option-metadata.js
@@ -113,7 +113,7 @@ exports.isMochaFlag = flag => {
  * @private
  */
 exports.expectedTypeForFlag = flag => {
-  const normalizedName = flag?.replace(/^--?/, '');
+  const normalizedName = flag.replace(/^--?/, '');
 
   // If flag is an alias, get it's full name.
   const aliases = exports.aliases;

From f894ed14396319eebba410f06b77825e74c3cc0c Mon Sep 17 00:00:00 2001
From: Dinika Saxena <dinikasaxenas@gmail.com>
Date: Wed, 4 Dec 2024 17:39:56 +0100
Subject: [PATCH 10/13] Add test cases for empty string checks for isNumeric

---
 test/node-unit/utils.spec.js | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/test/node-unit/utils.spec.js b/test/node-unit/utils.spec.js
index d910de496c..06fe287bf2 100644
--- a/test/node-unit/utils.spec.js
+++ b/test/node-unit/utils.spec.js
@@ -55,6 +55,15 @@ describe('utils', function () {
       it('returns false for a string that cannot be parsed as a number', function () {
         expect(utils.isNumeric('foo'), 'to equal', false);
       });
+      it('returns false for empty string', function () {
+        expect(utils.isNumeric(''), 'to equal', false);
+      });
+      it('returns false for empty string with many whitespaces', function () {
+        expect(utils.isNumeric('    '), 'to equal', false);
+      });
+      it('returns true for stringified zero', function () {
+        expect(utils.isNumeric('0'), 'to equal', true);
+      });
     });
   });
 });

From 19bd2a4746a377f82e3006d5300d5a4379474722 Mon Sep 17 00:00:00 2001
From: Dinika Saxena <dinikasaxenas@gmail.com>
Date: Thu, 5 Dec 2024 11:57:13 +0100
Subject: [PATCH 11/13] Do not add extra leading -- in error message for flag

---
 lib/cli/options.js                 | 2 +-
 test/node-unit/cli/options.spec.js | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/cli/options.js b/lib/cli/options.js
index ed350ce82f..8ef5a28c6b 100644
--- a/lib/cli/options.js
+++ b/lib/cli/options.js
@@ -133,7 +133,7 @@ const parse = (args = [], defaultValues = {}, ...configObjects) => {
       expectedTypeForFlag(maybeFlag) !== 'number'
     ) {
       throw createInvalidArgumentTypeError(
-        `Mocha flag '--${maybeFlag}' given invalid option: '${arg}'`,
+        `Mocha flag '${maybeFlag}' given invalid option: '${arg}'`,
         Number(arg),
         expectedTypeForFlag(maybeFlag)
       );
diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js
index a99812d328..2b6200fbcc 100644
--- a/test/node-unit/cli/options.spec.js
+++ b/test/node-unit/cli/options.spec.js
@@ -690,7 +690,7 @@ describe('options', function () {
 
       const invalidArgError = (flag, arg, expectedType = 'string') => {
         return {
-          message: `Mocha flag '--${flag}' given invalid option: '${arg}'`,
+          message: `Mocha flag '${flag}' given invalid option: '${arg}'`,
           code: constants.INVALID_ARG_TYPE,
           argument: arg,
           actual: 'number',

From 912bdd18681d46fd255aa7523fe426d51bdf2511 Mon Sep 17 00:00:00 2001
From: Dinika Saxena <dinikasaxenas@gmail.com>
Date: Thu, 5 Dec 2024 12:48:07 +0100
Subject: [PATCH 12/13] Throw error for numeric positional arg after
 yargs-parser has parsed args

Signed-off-by: Dinika Saxena <dinikasaxenas@gmail.com>
---
 lib/cli/options.js                 | 66 +++++++++++++++++++++---------
 test/node-unit/cli/options.spec.js |  8 ++++
 2 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/lib/cli/options.js b/lib/cli/options.js
index 8ef5a28c6b..26199093be 100644
--- a/lib/cli/options.js
+++ b/lib/cli/options.js
@@ -103,6 +103,44 @@ const nargOpts = types.array
   .concat(types.string, types.number)
   .reduce((acc, arg) => Object.assign(acc, {[arg]: 1}), {});
 
+/**
+ * Throws either "UNSUPPORTED" error or "INVALID_ARG_TYPE" error for numeric positional arguments.
+ * @param {string[]} allArgs - Stringified args passed to mocha cli
+ * @param {number} numericArg - Numeric positional arg for which error must be thrown
+ * @param {Object} parsedResult - Result from `yargs-parser`
+ * @private
+ * @ignore
+ */
+const createErrorForNumericPositionalArg = (
+  allArgs,
+  numericArg,
+  parsedResult
+) => {
+  // A flag for `numericArg` exists if:
+  // 1. A mocha flag immediately preceeded the numericArg in `allArgs` array and
+  // 2. `numericArg` value could not be assigned to this flag by `yargs-parser` because of incompatible datatype.
+  const flag = allArgs.find((arg, index) => {
+    const normalizedArg = arg.replace(/^--?/, '');
+    return (
+      isMochaFlag(arg) &&
+      allArgs[index + 1] === String(numericArg) &&
+      parsedResult[normalizedArg] !== numericArg
+    );
+  });
+
+  if (flag) {
+    throw createInvalidArgumentTypeError(
+      `Mocha flag '${flag}' given invalid option: '${numericArg}'`,
+      numericArg,
+      expectedTypeForFlag(flag)
+    );
+  } else {
+    throw createUnsupportedError(
+      `Option ${numericArg} is unsupported by the mocha cli`
+    );
+  }
+};
+
 /**
  * Wrapper around `yargs-parser` which applies our settings
  * @param {string|string[]} args - Arguments to parse
@@ -120,25 +158,6 @@ const parse = (args = [], defaultValues = {}, ...configObjects) => {
   // 4. we can then reapply the values after yargs-parser is done.
   const allArgs = Array.isArray(args) ? args : args.split(' ');
   const nodeArgs = allArgs.reduce((acc, arg, index, allArgs) => {
-    const maybeFlag = allArgs[index - 1];
-
-    if (isNumeric(arg) && (!maybeFlag || !isMochaFlag(maybeFlag))) {
-      throw createUnsupportedError(
-        `Option ${arg} is unsupported by the mocha cli`
-      );
-    }
-    if (
-      isNumeric(arg) &&
-      isMochaFlag(maybeFlag) &&
-      expectedTypeForFlag(maybeFlag) !== 'number'
-    ) {
-      throw createInvalidArgumentTypeError(
-        `Mocha flag '${maybeFlag}' given invalid option: '${arg}'`,
-        Number(arg),
-        expectedTypeForFlag(maybeFlag)
-      );
-    }
-
     const pair = arg.split('=');
     let flag = pair[0];
     if (isNodeFlag(flag, false)) {
@@ -165,6 +184,15 @@ const parse = (args = [], defaultValues = {}, ...configObjects) => {
     process.exit(1);
   }
 
+  const numericPositionalArg = result.argv._.find(arg => isNumeric(arg));
+  if (numericPositionalArg) {
+    createErrorForNumericPositionalArg(
+      allArgs,
+      numericPositionalArg,
+      result.argv
+    );
+  }
+
   // reapply "=" arg values from above
   nodeArgs.forEach(([key, value]) => {
     result.argv[key] = value;
diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js
index 2b6200fbcc..cb456869ae 100644
--- a/test/node-unit/cli/options.spec.js
+++ b/test/node-unit/cli/options.spec.js
@@ -771,6 +771,14 @@ describe('options', function () {
           'not to throw'
         );
       });
+
+      it('does not throw error if numeric value is passed to string flag', function () {
+        expect(() => loadOptions(`--grep ${numericArg}`), 'not to throw');
+      });
+
+      it('does not throw error if numeric value is passed to an array flag', function () {
+        expect(() => loadOptions(`--spec ${numericArg}`), 'not to throw');
+      });
     });
   });
 });

From 163dd91ecfcb3976084c3a833276ae21d6b8e4b6 Mon Sep 17 00:00:00 2001
From: Dinika Saxena <dinikasaxenas@gmail.com>
Date: Thu, 5 Dec 2024 13:20:36 +0100
Subject: [PATCH 13/13] Revert timeout and slow as string flags so that they
 can accept human readable values

Signed-off-by: Dinika Saxena <dinikasaxenas@gmail.com>
---
 lib/cli/options.js                 |  8 ++++----
 lib/cli/run-option-metadata.js     | 15 ++++++++++++---
 test/node-unit/cli/options.spec.js |  2 +-
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/lib/cli/options.js b/lib/cli/options.js
index 26199093be..09351957b4 100644
--- a/lib/cli/options.js
+++ b/lib/cli/options.js
@@ -112,8 +112,8 @@ const nargOpts = types.array
  * @ignore
  */
 const createErrorForNumericPositionalArg = (
-  allArgs,
   numericArg,
+  allArgs,
   parsedResult
 ) => {
   // A flag for `numericArg` exists if:
@@ -124,7 +124,7 @@ const createErrorForNumericPositionalArg = (
     return (
       isMochaFlag(arg) &&
       allArgs[index + 1] === String(numericArg) &&
-      parsedResult[normalizedArg] !== numericArg
+      parsedResult[normalizedArg] !== String(numericArg)
     );
   });
 
@@ -157,7 +157,7 @@ const parse = (args = [], defaultValues = {}, ...configObjects) => {
   //    are ALL boolean flags.
   // 4. we can then reapply the values after yargs-parser is done.
   const allArgs = Array.isArray(args) ? args : args.split(' ');
-  const nodeArgs = allArgs.reduce((acc, arg, index, allArgs) => {
+  const nodeArgs = allArgs.reduce((acc, arg) => {
     const pair = arg.split('=');
     let flag = pair[0];
     if (isNodeFlag(flag, false)) {
@@ -187,8 +187,8 @@ const parse = (args = [], defaultValues = {}, ...configObjects) => {
   const numericPositionalArg = result.argv._.find(arg => isNumeric(arg));
   if (numericPositionalArg) {
     createErrorForNumericPositionalArg(
-      allArgs,
       numericPositionalArg,
+      allArgs,
       result.argv
     );
   }
diff --git a/lib/cli/run-option-metadata.js b/lib/cli/run-option-metadata.js
index 0aabd2ad93..df967097a7 100644
--- a/lib/cli/run-option-metadata.js
+++ b/lib/cli/run-option-metadata.js
@@ -50,8 +50,17 @@ const TYPES = (exports.types = {
     'sort',
     'watch'
   ],
-  number: ['retries', 'jobs', 'slow', 'timeout'],
-  string: ['config', 'fgrep', 'grep', 'package', 'reporter', 'ui']
+  number: ['retries', 'jobs'],
+  string: [
+    'config',
+    'fgrep',
+    'grep',
+    'package',
+    'reporter',
+    'ui',
+    'slow',
+    'timeout'
+  ]
 });
 
 /**
@@ -108,7 +117,7 @@ exports.isMochaFlag = flag => {
 
 /**
  * Returns expected yarg option type for a given mocha flag.
- * @param {string} flag - Flag to check (can be with out without leading "--"")
+ * @param {string} flag - Flag to check (can be with or without leading dashes "--"")
  * @returns {string | undefined} - If flag is a valid mocha flag, the expected type of argument for this flag is returned, otherwise undefined is returned.
  * @private
  */
diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js
index cb456869ae..f5ce73da15 100644
--- a/test/node-unit/cli/options.spec.js
+++ b/test/node-unit/cli/options.spec.js
@@ -527,7 +527,7 @@ describe('options', function () {
           loadOptions('--timeout 500'),
           'to have property',
           'timeout',
-          500
+          '500'
         );
       });