fixing process.nexTick (should be process.nextTick), which invalidated tests

[jsoref]

Requirements

Description of the Change

This is intended to fix a spelling error -- Split from #2981.

Alternate Designs

Instead of fixing the spelling error, the misspelled token can be replaced by a clear call to a nonexistent (but properly spelled) method.

Why should this be in core?

This is existing test code (that happens to be incorrectly documenting what it claims to be testing).

Benefits

Once someone fixes this to work correctly it will be easier to understand what the test is testing.

Possible Drawbacks

process.nexTick could be intentionally misspelled, but that's unlikely, if that's the goal, there are better ways to cause a similar error.

Applicable issues

• Is it a bug fix, or does it not impact production code (patch release)?
  This isn't a proper bug fix at this time. Someone will have to decide how to properly fix it.

[ScottFreeCode]

Thanks for splitting this out!

If we can figure out what's causing test failures and address it, then I'm definitely comfortable merging this fix to the tests. It would be nice to ensure they wouldn't have passed with the incorrect spelling in the first place, but the fact is that before this fix they're not even testing what they're supposed to and with this fix they are, so it's an unambiguous step in the right direction. Especially if there's a regression that we missed detecting before due to the incorrect spelling...

[jsoref]

Perhaps this needs this.allowUncaught. From there, I think I'd need to use node-inspector to figure out where to go next...

[boneskull]

I don't quite understand why we need allowUncaught?

[jsoref]

It was a stab in the dark. And it didn't work.

[jsoref]

ok, node-inspector was a total dead-end. But I managed to get node --inspect to talk to me. It wasn't particularly helpful... This version works.

We treat Runner.prototype.uncaught as something we expect to have called -- because it should be called.

I had to move the done() call earlier because otherwise, I hit a timeout.

I have no idea why one pair of seemingly unrelated tests is failing on one specific subplatform of one of two test systems.

[coveralls]

Coverage increased (+0.1%) to 89.156% when pulling 4241900 on jsoref:next-broken into aa52933 on mochajs:master.

[ScottFreeCode]

This is definitely not right -- we don't expect users to monkey-patch Runner.prototype.uncaught to suppress its behavior here, and toBeCalled is not a member of the expect library used in Mocha's tests.

[jsoref]

Ok. I'm not going to spend any more time on this. There's no obvious way to make uncaught cooperate. I tried setting allowUncaught on various things and stepping through, it didn't seem to help. And uncaught doesn't have any hooks.

The to be called thing didn't match things, but it also didn't die which confused me... Anyway, good luck. I'm off to other projects.

[ScottFreeCode]

Ok, that's fine -- thanks for bringing the faulty tests to our attention nonetheless! We'll see if we can track down what's wrong in the source (if fiddling with Runner.prototype.uncaught works around it, then that function seems like a good place to start looking), and try to make sure Git/GitHub credits you for starting the fix when we do figure it out!

[boneskull]

fwiw, I think I've figured out why this happens.

Look at this thing:

    it('should not pass if throwing async and test is async', function (done) {
      var test = new Test('im async and throw null async', function (done2) {
        process.nextTick(function () {
          throw null;
        });
      });
      // blah blah, run the test, and finish up
    });

What happens when an asynchronous exception is thrown in Node.js? Well, the uncaughtException event is emitted, and any handlers are executed. It doesn't care which callback your error originates in.

When do we register an uncaughtException event handler? When the Runner runs. Because we're in the middle of a test file, we have a Runner instance already--with its own uncaughtException handler. Creating another Runner instance doesn't magically make the first uncaughtException handler disappear.

In other words, the Test and Runner within our test works as expected. But the uncaught exception reaches the main thread (while "should not pass if throwing async and test is async" is the current Runnable) and so Mocha handles it as it should.

Potential solutions:

1. Make this an integration test instead, which spawns a subprocess.
2. Temporarily remove any uncaughtException handlers.

I'm going to see if 2. works.

[boneskull]

closing in lieu of #3130 with fix applied.