Issue/2819 - Fix / Clarify this.skip behavior in before hooks (#3225)

* Fix Issue 2819 - this.skip should skip tests in nested suites.

Side Effects:
- Fixed typo in beforeEach async pending test that was using sync fixture.
- Fixed beforeEach async pending test to reflect current behavior.

* fixup! Fix Issue 2819 - this.skip should skip tests in nested suites.

* docs: Add more details regarding suite-level skip.

* test: Update error messages in before hook integration tests.

* test: Remove timeout delays from pending tests.

* test: Consistently throw errors in pending test fixtures.

docs/index.md

@@ -635,6 +635,30 @@ before(function() {
 });
 ```
 
+This will skip all `it`, `beforeEach/afterEach`, and `describe` blocks within the suite. `before/after` hooks are skipped unless they are defined at the same level as the hook containing `this.skip()`.
+
+```js
+describe('outer', function () {
+  before(function () {
+    this.skip();
+  });
+
+  after(function () {
+    // will be executed
+  });
+
+  describe('inner', function () {
+    before(function () {
+      // will be skipped
+    });
+
+    after(function () {
+      // will be skipped
+    });
+  });
+});
+```
+
 > Before Mocha v3.0.0, `this.skip()` was not supported in asynchronous tests and hooks.
 
 ## Retry Tests

lib/runner.js

@@ -319,6 +319,9 @@ Runner.prototype.hook = function(name, fn) {
             suite.tests.forEach(function(test) {
               test.pending = true;
             });
+            suite.suites.forEach(function(suite) {
+              suite.pending = true;
+            });
             // a pending hook won't be executed twice.
             hook.pending = true;
           }

test/integration/fixtures/pending/skip-async-before-hooks.fixture.js

@@ -0,0 +1,60 @@
+'use strict';
+
+describe('outer suite', function () {
+
+  before(function () {
+    console.log('outer before');
+  });
+
+  it('should run this test', function () { });
+
+  describe('inner suite', function () {
+
+    before(function (done) {
+      console.log('inner before');
+      var self = this;
+      setTimeout(function () {
+        self.skip();
+        done();
+      }, 0);
+    });
+
+    beforeEach(function () {
+      throw new Error('beforeEach should not run');
+    });
+
+    afterEach(function () {
+      throw new Error('afterEach should not run');
+    });
+
+    it('should not run this test', function () {
+      throw new Error('inner suite test should not run');
+    });
+
+    after(function () {
+      console.log('inner after');
+    });
+
+    describe('skipped suite', function () {
+      before(function () {
+        console.log('skipped before');
+      });
+
+      it('should not run this test', function () {
+        throw new Error('skipped suite test should not run');
+      });
+
+      after(function () {
+        console.log('skipped after');
+      });
+    });
+
+  });
+
+  it('should run this test', function () { });
+
+  after(function () {
+    console.log('outer after');
+  });
+
+});

test/integration/fixtures/pending/skip-async-before-nested.fixture.js

@@ -0,0 +1,41 @@
+describe('skip in before with nested describes', function () {
+  before(function (done) {
+    var self = this;
+    setTimeout(function () {
+      self.skip();
+      done();
+    }, 0);
+  });
+
+  it('should never run this test', function () {
+    throw new Error('never run this test');
+  });
+
+  describe('nested describe', function () {
+    before(function () {
+      throw new Error('first level before should not run');
+    });
+
+    it('should never run this test', function () {
+      throw new Error('never run this test');
+    });
+
+    after(function () {
+      throw new Error('first level after should not run');
+    });
+
+    describe('nested again', function () {
+      before(function () {
+        throw new Error('second level before should not run');
+      });
+
+      it('should never run this test', function () {
+        throw new Error('never run this test');
+      });
+
+      after(function () {
+        throw new Error('second level after should not run');
+      });
+    });
+  });
+});

test/integration/fixtures/pending/skip-async-before.fixture.js

@@ -1,18 +1,23 @@
 'use strict';
 
-describe('skip in before', function () {
-  before(function (done) {
-    var self = this;
-    setTimeout(function () {
-      self.skip();
-    }, 50);
-  });
+describe('outer describe', function () {
+  it('should run this test', function () {});
 
-  it('should never run this test', function () {
-    throw new Error('never thrown');
-  });
+  describe('skip in before', function () {
+    before(function (done) {
+      var self = this;
+      setTimeout(function () {
+        self.skip();
+      }, 0);
+    });
 
-  it('should never run this test', function () {
-    throw new Error('never thrown');
+    it('should never run this test', function () {
+      throw new Error('never run this test');
+    });
+    it('should never run this test', function () {
+      throw new Error('never run this test');
+    });
   });
+
+  it('should run this test', function () {});
 });

test/integration/fixtures/pending/skip-async-beforeEach.fixture.js

@@ -5,14 +5,17 @@ describe('skip in beforeEach', function () {
     var self = this;
     setTimeout(function () {
       self.skip();
-    }, 50);
+      done();
+    }, 0);
   });
 
-  it('should never run this test', function () {
-    throw new Error('never thrown');
+  it('should skip this test', function () {
+    throw new Error('never run this test');
   });
-
-  it('should never run this test', function () {
-    throw new Error('never thrown');
+  it('should not reach this test', function () {
+    throw new Error('never run this test');
+  });
+  it('should not reach this test', function () {
+    throw new Error('never run this test');
   });
 });

test/integration/fixtures/pending/skip-async-spec.fixture.js

@@ -5,10 +5,8 @@ describe('skip in test', function () {
     var self = this;
     setTimeout(function () {
       self.skip();
-    }, 50);
+    }, 0);
   });
 
-  it('should run other tests in the suite', function () {
-    // Do nothing
-  });
+  it('should run other tests in the suite', function () {});
 });

test/integration/fixtures/pending/skip-sync-before-hooks.fixture.js

@@ -0,0 +1,57 @@
+'use strict';
+
+describe('outer suite', function () {
+
+  before(function () {
+    console.log('outer before');
+  });
+
+  it('should run this test', function () { });
+
+  describe('inner suite', function () {
+    before(function () {
+      this.skip();
+    });
+
+    before(function () {
+      console.log('inner before');
+    });
+
+    beforeEach(function () {
+      throw new Error('beforeEach should not run');
+    });
+
+    afterEach(function () {
+      throw new Error('afterEach should not run');
+    });
+
+    after(function () {
+      console.log('inner after');
+    });
+
+    it('should never run this test', function () {
+      throw new Error('inner suite test should not run');
+    });
+
+    describe('skipped suite', function () {
+      before(function () {
+        console.log('skipped before');
+      });
+
+      it('should never run this test', function () {
+        throw new Error('skipped suite test should not run');
+      });
+
+      after(function () {
+        console.log('skipped after');
+      });
+    });
+  });
+
+  it('should run this test', function () { });
+
+  after(function () {
+    console.log('outer after');
+  })
+
+});

test/integration/fixtures/pending/skip-sync-before-nested.fixture.js

@@ -0,0 +1,39 @@
+'use strict';
+
+describe('skip in before with nested describes', function () {
+  before(function () {
+    this.skip();
+  });
+
+  it('should never run this test', function () {
+    throw new Error('never run this test');
+  });
+
+  describe('nested describe', function () {
+    before(function () {
+      throw new Error('first level before should not run');
+    });
+
+    it('should never run this test', function () {
+      throw new Error('never run this test');
+    });
+
+    after(function () {
+      throw new Error('first level after should not run');
+    });
+
+    describe('nested again', function () {
+      before(function () {
+        throw new Error('second level before should not run');
+      });
+
+      it('should never run this test', function () {
+        throw new Error('never run this test');
+      });
+
+      after(function () {
+        throw new Error('second level after should not run');
+      });
+    });
+  });
+});

test/integration/fixtures/pending/skip-sync-before.fixture.js

@@ -1,15 +1,20 @@
 'use strict';
 
-describe('skip in before', function () {
-  before(function () {
-    this.skip();
-  });
+describe('outer describe', function () {
+  it('should run this test', function () {});
 
-  it('should never run this test', function () {
-    throw new Error('never thrown');
-  });
+  describe('skip in before', function () {
+    before(function () {
+      this.skip();
+    });
 
-  it('should never run this test', function () {
-    throw new Error('never thrown');
+    it('should never run this test', function () {
+      throw new Error('never run this test');
+    });
+    it('should never run this test', function () {
+      throw new Error('never run this test');
+    });
   });
+
+  it('should run this test', function () {});
 });

test/integration/fixtures/pending/skip-sync-beforeEach.fixture.js

@@ -6,10 +6,10 @@ describe('skip in beforeEach', function () {
   });
 
   it('should never run this test', function () {
-    throw new Error('never thrown');
+    throw new Error('never run this test');
   });
 
   it('should never run this test', function () {
-    throw new Error('never thrown');
+    throw new Error('never run this test');
   });
 });

test/integration/fixtures/pending/skip-sync-spec.fixture.js

@@ -3,10 +3,8 @@
 describe('skip in test', function () {
   it('should skip immediately', function () {
     this.skip();
-    throw new Error('never thrown');
+    throw new Error('never run this test');
   });
 
-  it('should run other tests in the suite', function () {
-    // Do nothing
-  });
+  it('should run other tests in the suite', function () {});
 });