🚀 Feature: Throw on nested it call

[dimadk24]

Is your feature request related to a problem or a nice-to-have?? Please describe.
It would be nice for Mocha to throw if it founds a nested it block. Currently, such blocks are silently ignored (code inside nested block is not called), but developers mistakenly can assume that they are called and test something.

I'm talking about such code:

describe('some function', () => {
  it('should work parent', () => {
    console.log('parent it block')

    // Accidentally created nested it block
    it('should work nested', () => {
      console.log('nested it block')
      assert.equal([1, 2, 3].indexOf(4), -1)
    })
  })
})

In such a simple example it's easy to spot the error but if you have a complex test structure, it's easy to accidentally create such a structure. And you would think that tests really test your code unless you add logs in nested it block and find out that none of your tests are really called.

Example of a more complex test structure:

const sumWithCondition = (condition, input) => {
  if (condition === 1 || condition === 2) {
    return input + 1
  }
  return input + 2
}

const runTest = (input, expected) => {
  testConditions = [1, 2]

  testConditions.forEach(condition => {
    it('should work with condition', () => {
      console.log('called')
      expect(someFunction(condition, input)).to.be.deep.equal(expected)
    })
  })
}

describe('some complex function', () => {
  it('should work with input 1', () => {
    runTest(1, 2)
  })

  it('should work with input 2', () => {
    runTest(2, 3);
  })
})

Describe the solution you'd like
Throw an error if find nested it block

Describe alternatives you've considered
You could also call nested it blocks, but it would create a strange test structure and it would not be consistent with other test libraries (e.g. jest)

Additional context
In jest such feature was implemented in jestjs/jest#9828
repo to check out with mocha and jest: https://github.com/DimaDK02/mocha-opportunity-to-improve/
Run yarn jest to run jest on this test and yarn mocha to run mocha

[JacobLey]

I currently write tests with ui: 'exports' for this exact reason. I found I was often accidentally writing nested it, and not actually testing code.
The BDD interface seems much better for "dynamic" tests, like in the example above for iterating over a list of possible cases, and making individual tests for each.

Unfortunately it seems like this issue never got any love... Is there a good way to get maintainer attention?

I'd be happy to open a PR to fix as well. I suspect the naive solution is something like:

let isInTest = false;
it = (name, test) => {
    if (isInTest) {
       throw new Error('No nested it');
    }
    doTest(name, test, () => {
        isInTest = false;
    });
};

My understanding of "parallel" mode is that it would still work in that situation, but if really it is important to check if a "nested" it is invoked, something like Node.js async hooks might be a good place to start...

[binoche9]

+1 This seems like an easy win to help prevent developers from accidentally writing tests that never get invoked and was implemented in jasmine a long time ago

[JoshuaKGoldberg]

Ha, I'm surprised this is allowed. Confirmed it's a no-op at the moment.

it("but inside", function () {
  it("doesn't matter", () => {});
});

$ npx mocha test.spec.js


  ✔ but inside

  1 passing (2ms)

This would be a semver-major breaking change as you never know who's relying on this behavior.