Parameterized Tests Can Create Multiple beforeEach Executions

[josh-cain]

Prerequisites

• [ x ] Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
• [ 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 that you not install Mocha globally.

Description

In certain cases with dynamically generated tests (like the one below), the same beforeEach or afterEach hook will be fired n times for each test. I can see why, in that the suite is constructed by scanning/enumerating all of the beforeEach/afterEach in the current scope, and since the .forEach loop has no child scopes it simply builds the suite with a hook for every iteration.

However, this feels like really strange behavior. Could this maybe be a case for a parameterized mechanism?

Steps to Reproduce

Just run this to observe:

'use strict';

const expect = require('chai').expect;

function square(number) {
  return number * number;
}

describe('a duplicating child scenario', () => {

  beforeEach('before each root', () => {
    console.log('before each root operation');
  });

  afterEach('after each root', () => {
    console.log('after each root operation');
  });

  const testCases = [
    { description: 'test case 1', arg: 1, expected: 1 },
    { description: 'test case 2', arg: 2, expected: 4 }
  ];
  testCases.forEach((testCase) => {

      beforeEach('before each child', () => {
        console.log('before each child operation');
      });

      afterEach('after each root', () => {
        console.log('after each child operation');
      });

      it(`should square a number for ${testCase.description}`, () => {
        expect(square(testCase.arg)).to.equal(testCase.expected);
      });
  });

});

describe('a non-duplicating child scenario', () => {

  beforeEach('before each root', () => {
    console.log('before each root operation');
  });

  afterEach('after each root', () => {
    console.log('after each root operation');
  });

  const testCases = [
    { description: 'test case 1', arg: 1, expected: 1 },
    { description: 'test case 2', arg: 2, expected: 4 }
  ];
  testCases.forEach((testCase) => {

    describe('using a namespaced suite', () => {

      beforeEach('before each child', () => {
        console.log('before each child operation');
      });

      afterEach('after each root', () => {
        console.log('after each child operation');
      });

      it(`should square a number for ${testCase.description}`, () => {
        expect(square(testCase.arg)).to.equal(testCase.expected);
      });

    });
  });

});

Expected behavior:
Each beforeEach and afterEach hook inside the parameterized forEach ran once per test.

Actual behavior: [What actually happens]
Each beforeEach and afterEach hook inside the parameterized forEach ran twice per test. Have also tried with n items in the testCases array, and this will result in the beforeEach and afterEach methods being run n times per test case.

Sample output:

  a duplicating child scenario
before each root operation
before each child operation
before each child operation
    ✓ should square a number for test case 1
after each root operation
after each child operation
after each child operation

Reproduces how often:
Every time.

Versions

Tested on 3.3.0 and 6.2.0

Additional Information

nope.

[juergba]

@josh-cain thank you for this issue and your excellent test examples. I will have a look.

The order of the afterEach hooks is wrong as well, after each root operation should be the last hook to run.

[juergba]

Your first example describe('a duplicating child scenario', () => { }:
It does not fire the same hook twice, it actually creates two identical hooks with the same content. Which is exactly what your forEach loop is asking for.

Your second example describe('a non-duplicating child scenario', () => { }:
Again you are creating two identical beforeEach and two identical afterEach hooks. Since you are also creating two identical describe's (suites) 'using a namespaced suite', each suite has one pair of hooks and the output is as expected.

IMO the output of your examples is as expected and no bug.
If you declare the hooks outside of the forEach loop, they are declared (and fired) only once.

describe('a non-duplicating child scenario', () => {
    beforeEach('before each root', () => {
      console.log('before each root operation');
    });  
    afterEach('after each root', () => {
      console.log('after each root operation');
    });
  
    const testCases = [
      { description: 'test case 1', arg: 1, expected: 1 },
      { description: 'test case 2', arg: 2, expected: 4 }
    ];
    // describe('using a namespaced suite', () => {  // works with or without this suite level 
      beforeEach('before each child', () => {
        console.log('before each child operation');
      });
      afterEach('after each child', () => {
        console.log('after each child operation');
      });
      testCases.forEach((testCase) => {  
        it(`should square a number for ${testCase.description}`, () => {
          assert.equal(square(testCase.arg), testCase.expected);
        });
      });
    // });
});

[josh-cain]

@juergba - thanks for getting back! I see what you're saying, and totally agree with your evaluation. However, to someone who is new to mochajs (like me!), this has the potential to wind up as a foot-gun. How about I see about adding a little more to the existing docs on this use case?

[juergba]

What is your proposal regarding docs? Adding hooks to the example?

[josh-cain]

@juergba sure, or at least mention the algorithm used to construct hooks and the way it interacts with the parameterized for loop. I'll send a PR and we can continue discussion there?