Mocha runner throws if test.pending is set to true in the 'test' event

[karanjitsingh]

I have a script that hooks on to mocha events and dynamically determines which tests to run at runtime. I do this by setting args.pending = true in the test hook. 7.0.0 seems to break this behavior.

Here's a sample repro

function mochaRun() {

    const mochaLib = require('mocha');
    mochaLib.Suite.prototype.beforeAll = () => { };
    mochaLib.Suite.prototype.afterAll = () => { };
    mochaLib.Suite.prototype.beforeEach = () => { };
    mochaLib.Suite.prototype.afterEach = () => { };

    this.mocha = new mochaLib({});

    this.mocha.addFile("test.js");

    const runner = this.mocha.run();
    
    runner.setMaxListeners(20);

    runner.on('test', (args) => { args.pending = true; });
}

// Use domain to catch the error in mocha
const domain = require('domain');
const executionDomain = domain.create();

executionDomain.on('error', (err) => {
    console.error(err);
});

executionDomain.run(() => {
    mochaRun()
});

with sample test.js:

const assert = require('assert');
describe('suite a', () => {
    it('test case a1', () => {

    });
    it('test case a2', () => {
        assert.fail('failure');
    });
})

On running this mocha throws with the error Cannot read property 'parent' of undefined

  suite a
    - test case a1
TypeError: Cannot read property 'parent' of undefined
    at Runner.parents (E:\mocharepro\node_modules\mocha\lib\runner.js:501:16)
    at Runner.hookUp (E:\mocharepro\node_modules\mocha\lib\runner.js:475:41)
    at E:\mocharepro\node_modules\mocha\lib\runner.js:658:21
    at next (E:\mocharepro\node_modules\mocha\lib\runner.js:450:14)
    at E:\mocharepro\node_modules\mocha\lib\runner.js:460:7
    at next (E:\mocharepro\node_modules\mocha\lib\runner.js:362:14)
    at Immediate.<anonymous> (E:\mocharepro\node_modules\mocha\lib\runner.js:428:5)
    at processImmediate (internal/timers.js:439:21)
    at process.topLevelDomainCallback (domain.js:130:23) {
  domainThrown: true
}

If this is now the expected behavior how do I get around it and dynamically determine which tests to skip.

[juergba]

@karanjitsingh could you please patch following line in "lib/runner.js":

• old: self.suite = errSuite;
• new: self.suite = errSuite || self.suite;

[juergba]

@karanjitsingh our CI tests haven't covered this test scenario, since they didn't fail in the past. So if you just open a PR with exactly my proposed line, our CI test have no significance, they don't prove wether the bug is fixed or not.

• either you test this patch in your environment and tell me wether it works or not
• or you add a test to your PR which covers this scenario
• or you do both items above

[karanjitsingh]

@juergba, Not sure how to test this change, if there is a test for that block of code please point me to it. Let me know if this needs to be a unit / functional test with helpful code pointers.

I can confirm that the change is working for me.

[juergba]

@karanjitsingh thank you.

I decided to open my own PR. At the moment we do not cover programmatic usage of Mocha in our CI tests. So adding a test case for this scenario was a little more complex.
I'm going to close your PR, sorry.

[karanjitsingh]

@juergba, np. What's the release cycle for such patches for mocha on npm?

[juergba]

We don't have a fix release cycle. Maybe I will give it a try this week-end.
I'm trying to set up my Windows10 / WSL 2.0 / Ubuntu environment ...