Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uncaughtException: fix recovery when current test is still running #4150

Merged
merged 4 commits into from
Jan 23, 2020

Conversation

juergba
Copy link
Contributor

@juergba juergba commented Jan 13, 2020

Description

describe('uncaught', function() {
  it('throw delayed error', (done) => {
    setTimeout(() => {
      throw new Error('Whoops!');
    }, 10)
    setTimeout(done, 10);
  });
  it('should wait 15ms', (done) => {      
    setTimeout(done, 15);
  });
  it('test 3', () => { });

  afterEach(function() {
    console.log('inside afterEach:', this.currentTest.title);
  });
});

This bug produces very variable results as throwing TypeError and failing empty tests. In above example the output is complete nonsense.

uncaught
    1) throw delayed error                  // first test fails
    √ throw delayed error                   // then same test passes
inside afterEach: throw delayed error      // same hook twice 
inside afterEach: throw delayed error
    √ test 3                                // second test is missing
    √ test 3                                // third test runs twice
inside afterEach: test 3                   // same hook twice 
inside afterEach: test 3

  3 passing (41ms)
  1 failing

  1) uncaught
        throw delayed error:
      Uncaught Error: Whoops!
      [...]

Fixed output:

uncaught
    1) throw delayed error
    2) throw delayed error
inside afterEach: throw delayed error
    √ should wait 15ms
inside afterEach: should wait 15ms
    √ test 3
inside afterEach: test 3

  2 passing (62ms)
  2 failing

  1) uncaught
       throw delayed error:
      Uncaught Error: Whoops!
      [...]
  2) uncaught
       throw delayed error:
      Error: done() called multiple times
      [...]

Description of the Change

We remove the recovery of a failing open test out of Runner#uncaught and pass the error to runnable's callback. done finishes the runnable and triggers the standard error handling incl. hook pattern.

Applicable issues

closes #2906, see comment

@juergba juergba self-assigned this Jan 13, 2020
@juergba juergba added area: async related to asynchronous use of Mocha type: bug a defect, confirmed by a maintainer type: cleanup a refactor semver-patch implementation requires increase of "patch" version number; "bug fixes" labels Jan 13, 2020
@coveralls
Copy link

coveralls commented Jan 13, 2020

Coverage Status

Coverage remained the same at 92.857% when pulling 0250f67 on juergba/recover-uncaught into 9c10ada on master.

@juergba juergba force-pushed the juergba/recover-uncaught branch from fd68864 to 68727a8 Compare January 13, 2020 13:46
@juergba juergba force-pushed the juergba/recover-uncaught branch from 68727a8 to 3bdf7f1 Compare January 15, 2020 09:02
@juergba juergba marked this pull request as ready for review January 15, 2020 09:27
@juergba juergba force-pushed the juergba/recover-uncaught branch 2 times, most recently from 5c3a8cf to a8a635b Compare January 16, 2020 13:44
@juergba
Copy link
Contributor Author

juergba commented Jan 17, 2020

@weswigham your testing would be welcome.

@juergba juergba force-pushed the juergba/recover-uncaught branch 3 times, most recently from ef383b5 to 3467a38 Compare January 18, 2020 13:52
Copy link
Contributor

@craigtaub craigtaub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried locally and I like it. Code makes sense. Like the additional tests. LGTM

@juergba juergba added this to the next milestone Jan 19, 2020
@juergba juergba force-pushed the juergba/recover-uncaught branch from 3467a38 to 0250f67 Compare January 23, 2020 08:57
@juergba juergba merged commit c0f1d14 into master Jan 23, 2020
@juergba juergba deleted the juergba/recover-uncaught branch January 23, 2020 10:14
bors bot added a commit to Third-Culture-Software/bhima that referenced this pull request Jan 26, 2020
4129: Update mocha to the latest version 🚀 r=jniles a=greenkeeper[bot]


## The devDependency [mocha](https://github.com/mochajs/mocha) was updated from `6.2.2` to `7.0.1`.
This version is **not covered** by your **current version range**.

If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update.

---

**Publisher:** [juergba](https://www.npmjs.com/~juergba)
**License:** MIT

<details>
<summary>Release Notes for v7.0.1</summary>

<h1>7.0.1 / 2020-01-25</h1>
<h2><g-emoji class="g-emoji" alias="bug" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/1f41b.png">🐛</g-emoji> Fixes</h2>
<ul>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/issues/4165" data-hovercard-type="pull_request" data-hovercard-url="/mochajs/mocha/pull/4165/hovercard">#4165</a>: Fix exception when skipping tests programmatically (<a href="https://urls.greenkeeper.io/juergba"><strong>@juergba</strong></a>)</li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/issues/4153" data-hovercard-type="pull_request" data-hovercard-url="/mochajs/mocha/pull/4153/hovercard">#4153</a>: Restore backwards compatibility for <code>reporterOptions</code> (<a href="https://urls.greenkeeper.io/holm"><strong>@holm</strong></a>)</li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/issues/4150" data-hovercard-type="pull_request" data-hovercard-url="/mochajs/mocha/pull/4150/hovercard">#4150</a>: Fix recovery of an open test upon uncaught exception (<a href="https://urls.greenkeeper.io/juergba"><strong>@juergba</strong></a>)</li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/issues/4147" data-hovercard-type="pull_request" data-hovercard-url="/mochajs/mocha/pull/4147/hovercard">#4147</a>: Fix regression of leaking uncaught exception handler (<a href="https://urls.greenkeeper.io/juergba"><strong>@juergba</strong></a>)</li>
</ul>
<h2><g-emoji class="g-emoji" alias="book" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/1f4d6.png">📖</g-emoji> Documentation</h2>
<ul>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/issues/4146" data-hovercard-type="pull_request" data-hovercard-url="/mochajs/mocha/pull/4146/hovercard">#4146</a>: Update copyright &amp; trademark notices per OJSF (<a href="https://urls.greenkeeper.io/boneskull"><strong>@boneskull</strong></a>)</li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/issues/4140" data-hovercard-type="pull_request" data-hovercard-url="/mochajs/mocha/pull/4140/hovercard">#4140</a>: Fix broken links (<a href="https://urls.greenkeeper.io/KyoungWan"><strong>@KyoungWan</strong></a>)</li>
</ul>
<h2><g-emoji class="g-emoji" alias="nut_and_bolt" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/1f529.png">🔩</g-emoji> Other</h2>
<ul>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/issues/4133" data-hovercard-type="pull_request" data-hovercard-url="/mochajs/mocha/pull/4133/hovercard">#4133</a>: Print more descriptive error message (<a href="https://urls.greenkeeper.io/Zirak"><strong>@Zirak</strong></a>)</li>
</ul>
</details>

<details>
<summary>Commits</summary>
<p>The new version differs by 61 commits ahead by 61, behind by 17.</p>
<ul>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/d0f04e994f3e78939f0a947ef064881c7fec5188"><code>d0f04e9</code></a> <code>Release v7.0.1</code></li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/2277958e32f48bed10f0cb2ceaf01e7b8045af35"><code>2277958</code></a> <code>update CHANGELOG for v7.0.1 [ci skip]</code></li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/0be3f78491bbbcdc4dcea660ee7bfd557a225d9c"><code>0be3f78</code></a> <code>Fix exception when skipping tests programmatically  (#4165)</code></li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/c0f1d1456dbc068f0552a5ceaed0d9b95e940ce1"><code>c0f1d14</code></a> <code>uncaughtException: fix recovery when current test is still running (#4150)</code></li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/9c10adab3340abd8baff147cb595256234d88de6"><code>9c10ada</code></a> <code>Fix backwards compability break for reporterOptions</code></li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/a24683fd9273d0896a177d70c2368ada4f2c4882"><code>a24683f</code></a> <code>Throw a descriptive error when a non-function is given to a runnable (#4133)</code></li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/579fd09db39a55b44c1f553df05c918bc62867be"><code>579fd09</code></a> <code>update copyright &amp; trademark notices per OJSF; closes #4145</code></li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/0e1ccbb915ba8c2f73134af5bebd357f3329b9b7"><code>0e1ccbb</code></a> <code>Fix leaking global 'uncaughtException' handler (#4147)</code></li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/7d78f209c6a4f8ef4eba584fe10515fd3901830e"><code>7d78f20</code></a> <code>Broken links in docs (#4140)</code></li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/69339a3e7710a790b106b922ce53fcb87772f689"><code>69339a3</code></a> <code>Release v7.0.0</code></li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/99e085f1fb924deeb87290adb476f4e375e72392"><code>99e085f</code></a> <code>update CHANGELOG for v7.0.0 [ci skip]</code></li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/35cf39b14eae6dbd1fb364c215093095d5912ebc"><code>35cf39b</code></a> <code>Add reporter alias names to docs (#4127)</code></li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/3bd2d28bfc99b5f71efc9ef332ae9ac4a5d90de8"><code>3bd2d28</code></a> <code>Forbid this.skip() within afterAll hooks (#4136)</code></li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/24c22bef53e4539dd17b0d3b2123953bb8a3a883"><code>24c22be</code></a> <code>Fix hook pattern of this.skip() in beforeEach hooks (#3741)</code></li>
<li><a href="https://urls.greenkeeper.io/mochajs/mocha/commit/1412dc80d87d0479f7f1d60202da2b33c90eb939"><code>1412dc8</code></a> <code>XUnit reporter should handle exceptions during diff generation (#4068)</code></li>
</ul>
<p>There are 61 commits in total.</p>
<p>See the <a href="https://urls.greenkeeper.io/mochajs/mocha/compare/843a322f9e7724e4a75ceff1920caf24da94bdf2...d0f04e994f3e78939f0a947ef064881c7fec5188">full diff</a></p>
</details>

---

<details>
  <summary>FAQ and help</summary>

  There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html). If those don’t help, you can always [ask the humans behind Greenkeeper](https://github.com/greenkeeperio/greenkeeper/issues/new).
</details>

---


Your [Greenkeeper](https://greenkeeper.io) bot 🌴



Co-authored-by: greenkeeper[bot] <23040076+greenkeeper[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: async related to asynchronous use of Mocha semver-patch implementation requires increase of "patch" version number; "bug fixes" type: bug a defect, confirmed by a maintainer type: cleanup a refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicated failing test summary
3 participants