feat: detect illegally nested tests (#4525)

lib/interfaces/bdd.js

@@ -3,6 +3,8 @@
 var Test = require('../test');
 var EVENT_FILE_PRE_REQUIRE =
   require('../suite').constants.EVENT_FILE_PRE_REQUIRE;
+var errors = require('../errors');
+var createUnsupportedError = errors.createUnsupportedError;
 
 /**
  * BDD-style interface:
@@ -84,6 +86,9 @@ module.exports = function bddInterface(suite) {
       if (suite.isPending()) {
         fn = null;
       }
+
+      common.checkForNestedTest(suite, title);
+
       var test = new Test(title, fn);
       test.file = file;
       suite.addTest(test);

lib/interfaces/common.js

@@ -50,6 +50,21 @@ module.exports = function (suites, context, mocha) {
       };
     },
 
+    /**
+     * Check for nested test registration.
+     *
+     * @param {Suite} suite The suite to which a test is being added.
+     * @param {string} title The title of the test being added.
+     */
+    checkForNestedTest: function checkForNestedTest(suite, title) {
+      var currentRunnable = suite.rootSuite().currentRunnable;
+      if (currentRunnable) {
+        throw createUnsupportedError(
+          'Nested test "' + title + '" detected inside "' + currentRunnable.title + '".'
+        );
+      }
+    },
+
     /**
      * Execute before running tests.
      *

lib/interfaces/tdd.js

@@ -3,6 +3,8 @@
 var Test = require('../test');
 var EVENT_FILE_PRE_REQUIRE =
   require('../suite').constants.EVENT_FILE_PRE_REQUIRE;
+var errors = require('../errors');
+var createUnsupportedError = errors.createUnsupportedError;
 
 /**
  * TDD-style interface:
@@ -84,6 +86,9 @@ module.exports = function (suite) {
       if (suite.isPending()) {
         fn = null;
       }
+
+      common.checkForNestedTest(suite, title);
+
       var test = new Test(title, fn);
       test.file = file;
       suite.addTest(test);

lib/runner.js

@@ -190,6 +190,7 @@ class Runner extends EventEmitter {
     this.state = constants.STATE_IDLE;
     this.total = suite.total();
     this.failures = 0;
+
     /**
      * @type {Map<EventEmitter,Map<string,Set<EventListener>>>}
      */
@@ -200,6 +201,8 @@ class Runner extends EventEmitter {
           test.parent.tests && test.parent.tests.indexOf(test.retriedTest());
         if (idx > -1) test.parent.tests[idx] = test;
       }
+      // Clear current runnable for nested test detection when test ends
+      self.suite.rootSuite().currentRunnable = null;
       self.checkGlobals(test);
     });
     this.on(constants.EVENT_HOOK_END, function (hook) {
@@ -523,6 +526,8 @@ Runner.prototype.hook = function (name, fn) {
       return fn();
     }
     self.currentRunnable = hook;
+    // Store current runnable on root suite for nested test detection
+    self.suite.rootSuite().currentRunnable = hook;
 
     if (name === HOOK_TYPE_BEFORE_ALL) {
       hook.ctx.currentTest = hook.parent.tests[0];
@@ -835,6 +840,8 @@ Runner.prototype.runTests = function (suite, fn) {
         return hookErr(err, errSuite, false);
       }
       self.currentRunnable = self.test;
+      // Store current runnable on root suite for nested test detection
+      self.suite.rootSuite().currentRunnable = self.test;
       self.runTest(function (err) {
         test = self.test;
         // conditional skip within it

lib/suite.js

@@ -400,6 +400,21 @@ Suite.prototype.titlePath = function () {
   return result;
 };
 
+/**
+ * Return the root suite by traversing up the parent chain.
+ *
+ * @memberof Suite
+ * @public
+ * @return {Suite}
+ */
+Suite.prototype.rootSuite = function () {
+  var suite = this;
+  while (suite.parent) {
+    suite = suite.parent;
+  }
+  return suite;
+};
+
 /**
  * Return the total number of tests.
  *

test/integration/fixtures/nested-tests/bdd-async-nested.fixture.js

@@ -0,0 +1,37 @@
+'use strict';
+
+// BDD async nested test fixtures - should fail with nested test errors
+describe('Async Nested Tests', function() {
+  it('sync nested test', function() {
+    it('nested in sync', function() {
+      // Should fail immediately
+    });
+  });
+
+  it('callback nested test', function(done) {
+    setTimeout(function() {
+      it('nested in callback', function() {
+        // Should fail with uncaught error
+      });
+      done();
+    }, 10);
+  });
+
+  it('promise nested test', function() {
+    return new Promise(function(resolve) {
+      setTimeout(function() {
+        it('nested in promise', function() {
+          // Should fail with uncaught error
+        });
+        resolve();
+      }, 10);
+    });
+  });
+
+  it('async/await nested test', async function() {
+    await new Promise(resolve => setTimeout(resolve, 10));
+    it('nested in async', function() {
+      // Should fail
+    });
+  });
+});

test/integration/fixtures/nested-tests/bdd-hook-nested.fixture.js

@@ -0,0 +1,15 @@
+'use strict';
+
+// BDD hook nested test fixture - should fail with nested test error
+describe('Hook Nested Test Suite', function() {
+  before(function() {
+    // This should fail due to nested test inside hook
+    it('nested test in before hook', function() {
+      // This nested test should cause an error
+    });
+  });
+
+  it('normal test', function() {
+    // This should pass if no hook error
+  });
+});

test/integration/fixtures/nested-tests/bdd-nested.fixture.js

@@ -0,0 +1,19 @@
+'use strict';
+
+// BDD nested test fixture - should fail with nested test error
+describe('Parent Suite', function() {
+  it('normal test', function() {
+    // This should pass
+  });
+
+  it('outer test with nested test', function() {
+    // This should fail due to nested test
+    it('inner nested test', function() {
+      // This nested test should cause an error
+    });
+  });
+
+  it('another normal test', function() {
+    // This should not run due to previous failure
+  });
+});

test/integration/fixtures/nested-tests/tdd-nested.fixture.js

@@ -0,0 +1,19 @@
+'use strict';
+
+// TDD nested test fixture - should fail with nested test error
+suite('Parent Suite', function() {
+  test('normal test', function() {
+    // This should pass
+  });
+
+  test('outer test with nested test', function() {
+    // This should fail due to nested test
+    test('inner nested test', function() {
+      // This nested test should cause an error
+    });
+  });
+
+  test('another normal test', function() {
+    // This should not run due to previous failure
+  });
+});