Repository: mochajs/mocha

Problem statement:
Add timeout option to xits using arrow functions
<!--
Have you read Mocha's Code of Conduct? By filing an Issue, you are expected to comply with it, including treating everyone with respect: https://github.com/mochajs/mocha/blob/master/.github/CODE_OF_CONDUCT.md
For more, check out the Mocha Gitter chat room: https://gitter.im/mochajs/mocha
-->

### Prerequisites
<!--
Place an `x` between the square brackets on the lines below for every satisified prerequisite.
-->
* [x] Checked that your issue isn't already filed by cross referencing [issues with the `common mistake` label](https://github.com/mochajs/mocha/issues?utf8=%E2%9C%93&q=is%3Aissue%20label%3Acommon-mistake%20)
* [x] Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
* [x] 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
* [x] Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with:
`node node_modules/.bin/mocha --version`(Local) and `mocha --version`(Global). We recommend avoiding the use of globally installed Mocha.

### Description
<!--
[Description of the issue]
-->
When using the arrow function syntax to override the default timeout. You specifiy it on the return value of the it function.

    it("should override the timeout", () => { }).timeout(100);

But if we want to ignore the test by adding an x we get an exception, which is annoying.

### Steps to Reproduce

    // Throws
    xit("should override the timeout", () => { }).timeout(100);
<!--
Please add a series of steps to reproduce the problem. See https://stackoverflow.com/help/mcve for in depth information
on how to create a mimimal, complete, and verifiable example.
-->

**Expected behavior:** The test should be ignored

**Actual behavior:** We get an exception
>> Mocha` exploded!
>> TypeError: Cannot read property 'timeout' of undefined

**Reproduces how often:** 100%

### Versions
mocha 4.0.1




Hints:
Is this a documented API?  I don't recall ever seeing anyone use this before.
anyway, this looks pretty trivial to address.   I'm going to call it a bug even though it's more of an "internal consistency" issue, since we don't necessarily expect consumers to use the object returned from `it` or `xit` for anything.
@boneskull this is a very interesting side effect of returning the test object from inside the `it` function. I have a PR coming in a few.

Patch:
diff --git a/lib/interfaces/bdd.js b/lib/interfaces/bdd.js
--- a/lib/interfaces/bdd.js
+++ b/lib/interfaces/bdd.js
@@ -102,7 +102,7 @@ module.exports = function (suite) {
      */
 
     context.xit = context.xspecify = context.it.skip = function (title) {
-      context.it(title);
+      return context.it(title);
     };
 
     /**


Test patch:
diff --git a/test/integration/fixtures/pending/skip-shorthand.fixture.js b/test/integration/fixtures/pending/skip-shorthand.fixture.js
new file mode 100644
--- /dev/null
+++ b/test/integration/fixtures/pending/skip-shorthand.fixture.js
@@ -0,0 +1,7 @@
+'use strict';
+
+describe('pending shorthand', function () {
+  xit('pending spec', function () {}).timeout(0);
+  xspecify('pending spec', function () {}).timeout(0);
+  it.skip('pending spec', function () {}).timeout(0);
+});
diff --git a/test/integration/pending.spec.js b/test/integration/pending.spec.js
--- a/test/integration/pending.spec.js
+++ b/test/integration/pending.spec.js
@@ -19,6 +19,19 @@ describe('pending', function () {
         done();
       });
     });
+    it('should return the test object when used via shorthand methods', function (done) {
+      run('pending/skip-shorthand.fixture.js', args, function (err, res) {
+        if (err) {
+          done(err);
+          return;
+        }
+        assert.equal(res.stats.pending, 3);
+        assert.equal(res.stats.passes, 0);
+        assert.equal(res.stats.failures, 0);
+        assert.equal(res.code, 0);
+        done();
+      });
+    });
   });
 
   describe('synchronous skip()', function () {
diff --git a/test/interfaces/bdd.spec.js b/test/interfaces/bdd.spec.js
--- a/test/interfaces/bdd.spec.js
+++ b/test/interfaces/bdd.spec.js
@@ -40,3 +40,27 @@ describe('pending suite', function () {
     });
   });
 });
+
+describe('pending tests', function () {
+  it.skip('should not run', function () {
+    expect(1 + 1).to.equal(3);
+  });
+});
+
+describe('setting timeout by appending it to test', function () {
+  var runningTest = it('enables users to call timeout on active tests', function () {
+    expect(1 + 1).to.equal(2);
+  }).timeout(1003);
+
+  var skippedTest = xit('enables users to call timeout on pending tests', function () {
+    expect(1 + 1).to.equal(3);
+  }).timeout(1002);
+
+  it('sets timeout on pending tests', function () {
+    expect(skippedTest._timeout).to.equal(1002);
+  });
+
+  it('sets timeout on running tests', function () {
+    expect(runningTest._timeout).to.equal(1003);
+  });
+});