-
Notifications
You must be signed in to change notification settings - Fork 779
Ensure unhandled rejections cause test failure. #1241
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
Merged
trentmwillis
merged 8 commits into
qunitjs:master
from
rwjblue:ensure-unhandled-rejections-fail
Dec 20, 2017
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
336b32e
CLI: Ensure an unhandled rejection results in a failed test.
bc23a65
HTML Reporter: Ensure unhandled rejection fails.
42eebf7
Core: Add QUnit.onUnhandledRejection.
f3e30f4
Tests: Update tests for unhandled rejection scenarios.
9a77ca4
Core: Capture the proper stack trace in onUnhandledRejection.
6a6fae8
Tests: Update CLI test and test fixture to remove stacktrace ambiguity.
455ac83
Tests: Fixup comment indentation.
4b955d1
Tests: Address feedback, use more idiomatic promise chaining.
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| import { test } from "../test"; | ||
|
|
||
| import config from "./config"; | ||
| import { extend } from "./utilities"; | ||
| import { sourceFromStacktrace } from "./stacktrace"; | ||
|
|
||
| // Handle an unhandled rejection | ||
| export default function onUnhandledRejection( reason ) { | ||
| const resultInfo = { | ||
| result: false, | ||
| message: reason.message || "error", | ||
| actual: reason, | ||
| source: reason.stack || sourceFromStacktrace( 3 ) | ||
| }; | ||
|
|
||
| const currentTest = config.current; | ||
| if ( currentTest ) { | ||
| currentTest.assert.pushResult( resultInfo ); | ||
| } else { | ||
| test( "global failure", extend( function( assert ) { | ||
| assert.pushResult( resultInfo ); | ||
| }, { validTest: true } ) ); | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| "use strict"; | ||
|
|
||
| QUnit.module( "Unhandled Rejections", function() { | ||
| QUnit.test( "test passes just fine, but has a rejected promise", function( assert ) { | ||
| assert.ok( true ); | ||
|
|
||
| const done = assert.async(); | ||
|
|
||
| Promise.resolve().then( function() { | ||
|
|
||
| // throwing a non-Error here because stack trace representation | ||
| // across Node versions is not stable (they continue to get better) | ||
| throw { | ||
| message: "Error thrown in non-returned promise!", | ||
| stack: `Error: Error thrown in non-returned promise! | ||
| at /some/path/wherever/unhandled-rejection.js:13:11` | ||
| }; | ||
| } ); | ||
|
|
||
| // prevent test from exiting before unhandled rejection fires | ||
| setTimeout( done, 10 ); | ||
| } ); | ||
|
|
||
| // rejecting with a non-Error here because stack trace representation | ||
| // across Node versions is not stable (they continue to get better) | ||
| Promise.reject( { | ||
| message: "outside of a test context", | ||
| stack: `Error: outside of a test context | ||
| at Object.<anonymous> (/some/path/wherever/unhandled-rejection.js:20:18)` | ||
| } ); | ||
| } ); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| // Detect if the current browser supports `onunhandledrejection` (avoiding | ||
| // errors for browsers without the capability) | ||
| var HAS_UNHANDLED_REJECTION_HANDLER = "onunhandledrejection" in window; | ||
|
|
||
| if ( HAS_UNHANDLED_REJECTION_HANDLER ) { | ||
| QUnit.module( "Unhandled Rejections inside test context", function( hooks ) { | ||
| hooks.beforeEach( function( assert ) { | ||
| var originalPushResult = assert.pushResult; | ||
| assert.pushResult = function( resultInfo ) { | ||
|
|
||
| // Inverts the result so we can test failing assertions | ||
| resultInfo.result = !resultInfo.result; | ||
| originalPushResult( resultInfo ); | ||
| }; | ||
| } ); | ||
|
|
||
| QUnit.test( "test passes just fine, but has a rejected promise", function( assert ) { | ||
| const done = assert.async(); | ||
|
|
||
| Promise.resolve().then( function() { | ||
| throw new Error( "Error thrown in non-returned promise!" ); | ||
| } ); | ||
|
|
||
| // prevent test from exiting before unhandled rejection fires | ||
| setTimeout( done, 10 ); | ||
| } ); | ||
|
|
||
| } ); | ||
|
|
||
| QUnit.module( "Unhandled Rejections outside test context", function( hooks ) { | ||
| var originalPushResult; | ||
|
|
||
| hooks.beforeEach( function( assert ) { | ||
|
|
||
| // Duck-punch pushResult so we can check test name and assert args. | ||
| originalPushResult = assert.pushResult; | ||
|
|
||
| assert.pushResult = function( resultInfo ) { | ||
|
|
||
| // Restore pushResult for this assert object, to allow following assertions. | ||
| this.pushResult = originalPushResult; | ||
|
|
||
| this.strictEqual( this.test.testName, "global failure", "Test is appropriately named" ); | ||
|
|
||
| this.deepEqual( | ||
| resultInfo, | ||
| { | ||
| message: "Error message", | ||
| source: "filePath.js:1", | ||
| result: false, | ||
| actual: { | ||
| message: "Error message", | ||
| fileName: "filePath.js", | ||
| lineNumber: 1, | ||
| stack: "filePath.js:1" | ||
| } | ||
| }, | ||
| "Expected assert.pushResult to be called with correct args" | ||
| ); | ||
| }; | ||
| } ); | ||
|
|
||
| hooks.afterEach( function() { | ||
| QUnit.config.current.pushResult = originalPushResult; | ||
| } ); | ||
|
|
||
| // Actual test (outside QUnit.test context) | ||
| QUnit.onUnhandledRejection( { | ||
| message: "Error message", | ||
| fileName: "filePath.js", | ||
| lineNumber: 1, | ||
| stack: "filePath.js:1" | ||
| } ); | ||
| } ); | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is
reasonjust anErrorinstance?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No,
reasonis whatever the rejection value is. AnErrorinstance is almost certainly the most common thing thatreasonwould be, but it could also be a failed XHR, a string,undefined,null, etc. The value here is either whatever the argument toreject()or the argument tothrowis.See small demo JSBin with the following content:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay, makes sense. Early-morning me couldn't quite make that connection.