From 299a710071d1cdd57c7708faefaa4c26a6896a0f Mon Sep 17 00:00:00 2001
From: Christopher Hiller <boneskull@boneskull.com>
Date: Tue, 12 Mar 2019 11:57:53 -0700
Subject: [PATCH] fix timeout/slow string values and duplicate arguments

---
 lib/cli/run-option-metadata.js                |  4 +-
 lib/cli/run.js                                | 10 +++++
 .../fixtures/options/slow-test.fixture.js     | 11 +++++
 test/integration/invalid-arguments.spec.js    | 45 +++++++++++++------
 test/integration/options/timeout.spec.js      | 35 +++++++++++++++
 test/node-unit/cli/options.spec.js            |  4 +-
 6 files changed, 92 insertions(+), 17 deletions(-)
 create mode 100644 test/integration/fixtures/options/slow-test.fixture.js
 create mode 100644 test/integration/options/timeout.spec.js

diff --git a/lib/cli/run-option-metadata.js b/lib/cli/run-option-metadata.js
index 0838e6050c..7b94b18a82 100644
--- a/lib/cli/run-option-metadata.js
+++ b/lib/cli/run-option-metadata.js
@@ -44,8 +44,8 @@ exports.types = {
     'sort',
     'watch'
   ],
-  number: ['retries', 'slow', 'timeout'],
-  string: ['fgrep', 'grep', 'package', 'reporter', 'ui']
+  number: ['retries'],
+  string: ['fgrep', 'grep', 'package', 'reporter', 'ui', 'slow', 'timeout']
 };
 
 /**
diff --git a/lib/cli/run.js b/lib/cli/run.js
index 3a6ba0072c..832f7235ae 100644
--- a/lib/cli/run.js
+++ b/lib/cli/run.js
@@ -258,6 +258,16 @@ exports.builder = yargs =>
         }
       });
 
+      types.boolean.concat(types.string, types.number).forEach(opt => {
+        if (Array.isArray(argv[opt])) {
+          throw createInvalidArgumentValueError(
+            `Argument "${opt}" can only be provided once`,
+            opt,
+            argv[opt]
+          );
+        }
+      });
+
       // yargs.implies() isn't flexible enough to handle this
       if (argv.invert && !('fgrep' in argv || 'grep' in argv)) {
         throw createMissingArgumentError(
diff --git a/test/integration/fixtures/options/slow-test.fixture.js b/test/integration/fixtures/options/slow-test.fixture.js
new file mode 100644
index 0000000000..01fe3d472b
--- /dev/null
+++ b/test/integration/fixtures/options/slow-test.fixture.js
@@ -0,0 +1,11 @@
+'use strict';
+
+describe('a suite', function() {
+  it('should succeed, slowly', function(done) {
+    setTimeout(done, 500);
+  });
+
+  it('should succeed, very slowly', function(done) {
+    setTimeout(done, 1500);
+  });
+});
diff --git a/test/integration/invalid-arguments.spec.js b/test/integration/invalid-arguments.spec.js
index 6c147bdb48..adb9eea1d0 100644
--- a/test/integration/invalid-arguments.spec.js
+++ b/test/integration/invalid-arguments.spec.js
@@ -3,18 +3,37 @@
 var invokeMocha = require('./helpers').invokeMocha;
 
 describe('invalid arguments', function() {
-  it('should exit with failure if arguments are invalid', function(done) {
-    invokeMocha(
-      ['--ui'],
-      function(err, result) {
-        if (err) {
-          return done(err);
-        }
-        expect(result, 'to have failed');
-        expect(result.output, 'to match', /not enough arguments/i);
-        done();
-      },
-      {stdio: 'pipe'}
-    );
+  describe('when argument is missing required value', function() {
+    it('should exit with failure', function(done) {
+      invokeMocha(
+        ['--ui'],
+        function(err, result) {
+          if (err) {
+            return done(err);
+          }
+          expect(result, 'to have failed');
+          expect(result.output, 'to match', /not enough arguments/i);
+          done();
+        },
+        {stdio: 'pipe'}
+      );
+    });
+  });
+
+  describe('when argument is provided twice', function() {
+    it('should exit with failure', function(done) {
+      invokeMocha(
+        ['--timeout', '600', '--timeout', '600'],
+        function(err, result) {
+          if (err) {
+            return done(err);
+          }
+          expect(result, 'to have failed');
+          expect(result.output, 'to match', /can only be provided once/i);
+          done();
+        },
+        {stdio: 'pipe'}
+      );
+    });
   });
 });
diff --git a/test/integration/options/timeout.spec.js b/test/integration/options/timeout.spec.js
new file mode 100644
index 0000000000..a1412a87b1
--- /dev/null
+++ b/test/integration/options/timeout.spec.js
@@ -0,0 +1,35 @@
+'use strict';
+
+var helpers = require('../helpers');
+var runMochaJSON = helpers.runMochaJSON;
+
+describe('--timeout', function() {
+  it('should allow human-readable string value', function(done) {
+    runMochaJSON('options/slow-test', ['--timeout', '1s'], function(err, res) {
+      if (err) {
+        done(err);
+        return;
+      }
+      expect(res, 'to have failed')
+        .and('to have passed test count', 1)
+        .and('to have failed test count', 1);
+      done();
+    });
+  });
+
+  it('should allow numeric value', function(done) {
+    runMochaJSON('options/slow-test', ['--timeout', '1000'], function(
+      err,
+      res
+    ) {
+      if (err) {
+        done(err);
+        return;
+      }
+      expect(res, 'to have failed')
+        .and('to have passed test count', 1)
+        .and('to have failed test count', 1);
+      done();
+    });
+  });
+});
diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js
index b849db5ab3..145093012c 100644
--- a/test/node-unit/cli/options.spec.js
+++ b/test/node-unit/cli/options.spec.js
@@ -624,7 +624,7 @@ describe('options', function() {
           findupSync
         });
 
-        expect(loadOptions(), 'to satisfy', {timeout: 800, require: ['foo']});
+        expect(loadOptions(), 'to satisfy', {timeout: '800', require: ['foo']});
       });
 
       it('should prioritize package.json over mocha.opts', function() {
@@ -687,7 +687,7 @@ describe('options', function() {
           loadOptions('--timeout 500'),
           'to have property',
           'timeout',
-          500
+          '500'
         );
       });
     });