Skip to content

🐛 Bug: Support setImmediate between hooks on Promises

Open

Description

Prerequisites

  • Checked that your issue isn't already filed by cross referencing issues with the common mistake label
  • 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.
  • '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
  • 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 avoiding the use of globally installed Mocha.

Description

I'm opening this issue as a result of an investigation into chaijs/chai-as-promised#173. However, there's a lot of unrelated noise in that thread, so I'll demonstrate the issue here independently of chai-as-promised.

Hooks typically wait until the next tick of the event loop before proceeding, per these lines. As a result, if a Promise with no rejection handler is declared in a hook prior to the test in which the rejection handler is added to the Promise, then a warning is typically generated about an unhandled promise rejection, followed by another warning about asynchronous handling of the promise rejection.

I used the word "typically" twice above because what I described doesn't apply to the final beforeEach prior to the test; unlike the other hooks, the final beforeEach proceeds immediately to the test, without waiting for the next tick of the event loop.

Steps to Reproduce

Example 1:

let promise;

describe("mySuite", function () {
  before(function () {
    promise = Promise.reject(new Error());
  });

  it("myTest", function () {
    promise.catch(function () {});
  });
});

Example 2:

let promise;

describe("mySuite", function () {
  beforeEach(function () {  // This is the only different line
    promise = Promise.reject(new Error());
  });

  it("myTest", function () {
    promise.catch(function () {});
  });
});

Expected behavior: Although the tests are meaningless, ideally I'd expect neither of the examples to generate any unhandled promise rejection warnings, or at least for both examples to result in the same behavior.

Actual behavior: Example 1 generates an unhandled promise rejection and Example 2 doesn't.

Example 1 output:

mocha

mySuite
(node:14219) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error
(node:14219) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
✓ myTest
(node:14219) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)

1 passing (11ms)

Example 2 output:

mocha

mySuite
✓ myTest

1 passing (7ms)

Reproduces how often: 100%

Versions

Node v8.5.0
Mocha v3.5.3

Additional Information

Changing this line to favor process.nextTick over global.setImmediate fixes the issue so that neither example generates a warning, and the Mocha test suite still passes. But I don't know what other consequences there are.

I think that in most situations this issue isn't a problem. Most promises being tested in the wild have (or should have) rejection handlers already registered, so no warning would be generated in those cases, despite the next tick between hooks. I just started digging in order to understand what was happening with chaijs/chai-as-promised#173. The example in that issue is completely contrived, as are the examples I provided here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    semver-patchimplementation requires increase of "patch" version number; "bug fixes"implementation requires increase of "patch" version number; "bug fixes"status: accepting prsMocha can use your help with this one!Mocha can use your help with this one!type: buga defect, confirmed by a maintainera defect, confirmed by a maintainer

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions