From 07de3c6fdea60c0a5279eefebb30002171dafcda Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Sun, 27 Jun 2021 18:25:32 +0100 Subject: [PATCH] Core: Fix late onerror handling == Background == Previously, QUnit.onError and QUnit.onUnhandledRejection could report global errors by synthesizing a new test, even after a run has ended. This is problematic when an errors ocurrs after all modules (and their hooks) have finished, and the overall test run has ended. The most immediate problem is that hooks having finished already, means it is illegal for a new test to start since "after" has already run. To protect against such illegal calls, the hooks object is emptied internally, and this new test causes an internal error: ``` TypeError: Cannot read property 'length' of undefined ``` This is not underlying problem though, but rather our internal safeguard working as intended. The higher-level problem is that there is no appropiate way to report a late error as a test since the run has already ended. The `QUnit.done()` callbacks have run, and the `runEnd` event has been emitted. == Approach == Instead of trying to report (late) errors as a test, only print them to `console.warn()`, which goes to stderr in Node.js. For the CLI, also remember that uncaught errors were found and use that to make sure we don't change exitCode back to zero (e.g. in case we have an uncaught error after the last test but before our `runEnd` callback is called). == Changes == * Generalise `QUnit.onUnhandledRejection` and re-use it for `window.onerror` (browser), and uncaught exceptions (CLI). * Fix broken use of `QUnit.onError` in `process.on( "uncaughtException" )`. This was passing the wrong parameters. Use the new onUncaughtException method instead. * Clarify that `QUnit.onError` is only for `window.onerror`. For now, keep its strange non-standard signature as-is (with the custom object parameter), but document this and its return value. * Remove the unused "..args" from `QUnit.onError`. This was only ever passed from one of our unit tests to give one extra argument (a string of "actual"), which then ended up passed as "actual" parameter to `pushFailure()`. We never used this in the actual onError binding, so remove this odd variadic construct for now. * Change `ProcessingQueue#done`, which is in charge of reporting the "No tests were run" error, to no longer rely on the way that `QUnit.onError` previously queued a late test. The first part of this function may run twice (same as before, once after an empty test run, and one more time after the synthetic test has finished and the queue is empty again). Change this so that we no longer assign `finished = true` in that first part. This means we will still support queueing of this one late test. But, since the quueue is empty, we do need to call `advance()` manually as otherwise it'd never get processed. Previously, `finished = true` was assigned first, which meant that `QUnit.onError` was adding a test under that condition. But this worked anyway because `Test#queue` internally had manual advancing exactly for this use case, which is also where we now emit a deprecation warning (to become an error in QUnit 3). Note that using this for anything other than the "No tests run" error was already unreliable since generally runEnd would have been emitted already. The "No tests run" test was exactly done from the one sweet spot where it was (and remains) safe because that threw an error and thus prevented runEnd from being emitted. Fixes https://github.com/qunitjs/qunit/issues/1377. Ref https://github.com/qunitjs/qunit/issues/1322. Ref https://github.com/qunitjs/qunit/issues/1446. --- docs/config/QUnit.onUncaughtException.md | 33 ++++++++++++++ src/assert.js | 23 +--------- src/cli/run.js | 40 ++++++++++------- src/core.js | 14 ++++-- src/core/on-uncaught-exception.js | 52 ++++++++++++++++++++++ src/core/on-unhandled-rejection.js | 25 ----------- src/core/onerror.js | 52 +++++++++++----------- src/core/processing-queue.js | 40 +++++++++-------- src/core/utilities.js | 22 +++++++++ src/html-reporter/html.js | 3 +- src/test.js | 20 ++++++--- test/cli/fixtures/expected/tap-outputs.js | 31 +++++++------ test/cli/fixtures/unhandled-rejection.js | 17 +------ test/cli/main.js | 13 +++--- test/main/assert.js | 11 +++++ test/onerror/inside-test.js | 44 +++++++++++------- test/onerror/outside-test.js | 54 +++++++++++++---------- test/reporter-html/unhandled-rejection.js | 19 +++----- 18 files changed, 304 insertions(+), 209 deletions(-) create mode 100644 docs/config/QUnit.onUncaughtException.md create mode 100644 src/core/on-uncaught-exception.js delete mode 100644 src/core/on-unhandled-rejection.js diff --git a/docs/config/QUnit.onUncaughtException.md b/docs/config/QUnit.onUncaughtException.md new file mode 100644 index 000000000..9ec885898 --- /dev/null +++ b/docs/config/QUnit.onUncaughtException.md @@ -0,0 +1,33 @@ +--- +layout: default +title: QUnit.onUncaughtException() +excerpt: Handle a global error. +categories: + - config +version_added: "unversioned" +--- + +`QUnit.onUncaughtException( error )` + +Handle a global error that should result in a failed test run. + +| name | description | +|------|-------------| +| `error` (any) | Usually an `Error` object, but any other thrown or rejected value may be given as well. | + +### Examples + +```js +const error = new Error( "Failed to reverse the polarity of the neutron flow" ); +QUnit.onUncaughtException( error ); +``` + +```js +process.on( "uncaughtException", QUnit.onUncaughtException ); +``` + +```js +window.addEventListener( "unhandledrejection", function( event ) { + QUnit.onUncaughtException( event.reason ); +} ); +``` diff --git a/src/assert.js b/src/assert.js index 9f6a62215..ddb589674 100644 --- a/src/assert.js +++ b/src/assert.js @@ -4,7 +4,7 @@ import { internalStop, resetTestTimeout } from "./test"; import Logger from "./logger"; import config from "./core/config"; -import { objectType, objectValues } from "./core/utilities"; +import { objectType, objectValues, errorString } from "./core/utilities"; import { sourceFromStacktrace } from "./core/stacktrace"; import { clearTimeout } from "./globals"; @@ -491,25 +491,4 @@ class Assert { // eslint-disable-next-line dot-notation Assert.prototype.raises = Assert.prototype[ "throws" ]; -/** - * Converts an error into a simple string for comparisons. - * - * @param {Error|Object} error - * @return {string} - */ -function errorString( error ) { - const resultErrorString = error.toString(); - - // If the error wasn't a subclass of Error but something like - // an object literal with name and message properties... - if ( resultErrorString.slice( 0, 7 ) === "[object" ) { - - // Based on https://es5.github.com/#x15.11.4.4 - const name = error.name ? String( error.name ) : "Error"; - return error.message ? `${name}: ${error.message}` : name; - } else { - return resultErrorString; - } -} - export default Assert; diff --git a/src/cli/run.js b/src/cli/run.js index c5e563d59..bf0ce3dd1 100644 --- a/src/cli/run.js +++ b/src/cli/run.js @@ -89,28 +89,34 @@ async function run( args, options ) { } } } catch ( e ) { - - // eslint-disable-next-line no-loop-func - QUnit.module( files[ i ], function() { - const loadFailureMessage = `Failed to load the test file with error:\n${e.stack}`; - QUnit.test( loadFailureMessage, function( assert ) { - assert.true( false, "should be able to load file" ); - } ); - } ); + const error = new Error( `Failed to load file ${files[ i ]}\n${e.name}: ${e.message}` ); + error.stack = e.stack; + QUnit.onUncaughtException( error ); } } - let running = true; - - // Listen for unhandled rejections, and call QUnit.onUnhandledRejection - process.on( "unhandledRejection", function( reason ) { - QUnit.onUnhandledRejection( reason ); + // The below handlers set exitCode directly, to make sure it is set even if the + // uncaught exception happens after the last test (shortly before "runEnd", or + // asynchronously after "runEnd" if the process is still running). + // + // The variable makes sure that if the uncaught exception is before "runEnd", + // or from another "runEnd" callback, it also won't turn the error code + // back into a success. + let uncaught = false; + + // Handle the unhandled + process.on( "unhandledRejection", ( reason, _promise ) => { + QUnit.onUncaughtException( reason ); + process.exitCode = 1; + uncaught = true; } ); - - process.on( "uncaughtException", function( error ) { - QUnit.onError( error ); + process.on( "uncaughtException", ( error, _origin ) => { + QUnit.onUncaughtException( error ); + process.exitCode = 1; + uncaught = true; } ); + let running = true; process.on( "exit", function() { if ( running ) { console.error( "Error: Process exited before tests finished running" ); @@ -130,7 +136,7 @@ async function run( args, options ) { QUnit.on( "runEnd", function setExitCode( data ) { running = false; - if ( data.testCounts.failed ) { + if ( data.testCounts.failed || uncaught ) { process.exitCode = 1; } else { process.exitCode = 0; diff --git a/src/core.js b/src/core.js index 08789337f..5cf3fc72b 100644 --- a/src/core.js +++ b/src/core.js @@ -18,8 +18,8 @@ import ProcessingQueue from "./core/processing-queue"; import SuiteReport from "./reports/suite"; import { on, emit } from "./events"; -import onError from "./core/onerror"; -import onUnhandledRejection from "./core/on-unhandled-rejection"; +import onWindowError from "./core/onerror"; +import onUncaughtException from "./core/on-uncaught-exception"; const QUnit = {}; export const globalSuite = new SuiteReport(); @@ -47,8 +47,8 @@ extend( QUnit, { is, objectType, on, - onError, - onUnhandledRejection, + onError: onWindowError, + onUncaughtException, pushFailure, assert: Assert.prototype, @@ -99,6 +99,12 @@ extend( QUnit, { }, + onUnhandledRejection: function( reason ) { + Logger.warn( "QUnit.onUnhandledRejection is deprecated and will be removed in QUnit 3.0." + + " Please use QUnit.onUncaughtException instead." ); + onUncaughtException( reason ); + }, + extend: function( ...args ) { Logger.warn( "QUnit.extend is deprecated and will be removed in QUnit 3.0." + " Please use Object.assign instead." ); diff --git a/src/core/on-uncaught-exception.js b/src/core/on-uncaught-exception.js new file mode 100644 index 000000000..bc1a7c5ca --- /dev/null +++ b/src/core/on-uncaught-exception.js @@ -0,0 +1,52 @@ +import config from "./config"; +import ProcessingQueue from "./processing-queue"; +import { sourceFromStacktrace } from "./stacktrace"; +import { extend, errorString } from "./utilities"; +import Logger from "../logger"; +import { test } from "../test"; + +/** + * Handle a global error that should result in a failed test run. + * + * Summary: + * + * - If there is a current test, it becomes a failed assertion. + * - If there is a current module, it becomes a failed test (and bypassing filters). + * Note that if we're before any other test or module, it will naturally + * become a global test. + * - If the overall test run has ended, the error is printed to `console.warn()`. + * + * @since 2.17.0 + * @param {Error|any} error + */ +export default function onUncaughtException( error ) { + const message = errorString( error ); + + // We could let callers specify an extra offset to add to the number passed to + // sourceFromStacktrace, in case they are a wrapper further away from the error + // handler, and thus reduce some noise in the stack trace. However, we're not + // doing this right now because it would almost never be used in practice given + // the vast majority of error values will be an Error object, and thus have a + // clean stack trace already. + const source = error.stack || sourceFromStacktrace( 2 ); + + if ( config.current ) { + config.current.assert.pushResult( { + result: false, + message: `global failure: ${message}`, + source + } ); + } else if ( !ProcessingQueue.finished ) { + test( "global failure", extend( function( assert ) { + assert.pushResult( { result: false, message, source } ); + }, { validTest: true } ) ); + } else { + + // TODO: Once supported in js-reporters and QUnit, use a TAP "bail" event. + // The CLI runner can use this to ensure a non-zero exit code, even if + // emitted after "runEnd" and before the process exits. + // The HTML Reporter can use this to renmder it on the page in a test-like + // block for easy discovery. + Logger.warn( `${message}\n${source}` ); + } +} diff --git a/src/core/on-unhandled-rejection.js b/src/core/on-unhandled-rejection.js deleted file mode 100644 index 458ca4f79..000000000 --- a/src/core/on-unhandled-rejection.js +++ /dev/null @@ -1,25 +0,0 @@ -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 } ) ); - } -} - diff --git a/src/core/onerror.js b/src/core/onerror.js index 3d24f5285..c2201e66c 100644 --- a/src/core/onerror.js +++ b/src/core/onerror.js @@ -1,31 +1,33 @@ -import { pushFailure, test } from "../test"; - import config from "./config"; -import { extend } from "./utilities"; +import onUncaughtException from "./on-uncaught-exception"; -// Handle an unhandled exception. By convention, returns true if further -// error handling should be suppressed and false otherwise. -// In this case, we will only suppress further error handling if the -// "ignoreGlobalErrors" configuration option is enabled. -export default function onError( error, ...args ) { - if ( config.current ) { - if ( config.current.ignoreGlobalErrors ) { - return true; - } - pushFailure( - error.message, - error.stacktrace || error.fileName + ":" + error.lineNumber, - ...args - ); - } else { - test( "global failure", extend( function() { - pushFailure( - error.message, - error.stacktrace || error.fileName + ":" + error.lineNumber, - ...args - ); - }, { validTest: true } ) ); +/** + * Handle a window.onerror error. + * + * If there is a current test that sets the internal `ignoreGlobalErrors` field + * (such as during `assert.throws()`), then the error is ignored and native + * error reporting is suppressed as well. This is because in browsers, an error + * can sometimes end up in `window.onerror` instead of in the local try/catch. + * This ignoring of errors does not apply to our general onUncaughtException + * method, nor to our `unhandledRejection` handlers, as those are not meant + * to receive an "expected" error during `assert.throws()`. + * + * @see + * @param {Object} details + * @param {string} details.message + * @param {string} details.fileName + * @param {number} details.lineNumber + * @param {string|undefined} [details.stacktrace] + * @return {bool} True if native error reporting should be suppressed. + */ +export default function onWindowError( details ) { + if ( config.current && config.current.ignoreGlobalErrors ) { + return true; } + const err = new Error( details.message ); + err.stack = details.stacktrace || details.fileName + ":" + details.lineNumber; + onUncaughtException( err ); + return false; } diff --git a/src/core/processing-queue.js b/src/core/processing-queue.js index d77914ab1..1457d6bc9 100644 --- a/src/core/processing-queue.js +++ b/src/core/processing-queue.js @@ -1,4 +1,5 @@ import config from "./config"; +import onUncaughtException from "./on-uncaught-exception"; import { generateHash, now @@ -159,33 +160,36 @@ function unitSamplerGenerator( seed ) { function done() { const storage = config.storage; - ProcessingQueue.finished = true; - const runtime = now() - config.started; const passed = config.stats.all - config.stats.bad; + // We have reached the end of the processing queue, but there is one more + // thing we'd like to report as a possible failing test. For this to work + // we need to call `onUncaughtException` before closing `ProcessingQueue.finished`. + // However, we do need to call `advance()` in order to resume the processing queue. + // Once this new test is finished processing, we'll reach `done` again, and + // that time the below condition should evaluate to false. if ( config.stats.testCount === 0 && config.failOnZeroTests === true ) { - + let error; if ( config.filter && config.filter.length ) { - throw new Error( `No tests matched the filter "${config.filter}".` ); - } - - if ( config.module && config.module.length ) { - throw new Error( `No tests matched the module "${config.module}".` ); - } - - if ( config.moduleId && config.moduleId.length ) { - throw new Error( `No tests matched the moduleId "${config.moduleId}".` ); - } - - if ( config.testId && config.testId.length ) { - throw new Error( `No tests matched the testId "${config.testId}".` ); + error = new Error( `No tests matched the filter "${config.filter}".` ); + } else if ( config.module && config.module.length ) { + error = new Error( `No tests matched the module "${config.module}".` ); + } else if ( config.moduleId && config.moduleId.length ) { + error = new Error( `No tests matched the moduleId "${config.moduleId}".` ); + } else if ( config.testId && config.testId.length ) { + error = new Error( `No tests matched the testId "${config.testId}".` ); + } else { + error = new Error( "No tests were run." ); } - throw new Error( "No tests were run." ); - + onUncaughtException( error ); + advance(); + return; } + ProcessingQueue.finished = true; + emit( "runEnd", globalSuite.end( true ) ); runLoggingCallbacks( "done", { passed, diff --git a/src/core/utilities.js b/src/core/utilities.js index 77a444ced..45c9077e3 100644 --- a/src/core/utilities.js +++ b/src/core/utilities.js @@ -157,3 +157,25 @@ export function generateHash( module, testName ) { return hex.slice( -8 ); } + +/** + * Converts an error into a simple string for comparisons. + * + * @param {Error|any} error + * @return {string} + */ +export function errorString( error ) { + + // Use String() instead of toString() to handle non-object values like undefined or null. + const resultErrorString = String( error ); + + // If the error wasn't a subclass of Error but something like + // an object literal with name and message properties... + if ( resultErrorString.slice( 0, 7 ) === "[object" ) { + + // Based on https://es5.github.io/#x15.11.4.4 + return ( error.name || "Error" ) + ( error.message ? `: ${error.message}` : "" ); + } else { + return resultErrorString; + } +} diff --git a/src/html-reporter/html.js b/src/html-reporter/html.js index 3a965fa40..a79ae0cd9 100644 --- a/src/html-reporter/html.js +++ b/src/html-reporter/html.js @@ -1088,8 +1088,7 @@ export function escapeText( s ) { return ret; }; - // Listen for unhandled rejections, and call QUnit.onUnhandledRejection window.addEventListener( "unhandledrejection", function( event ) { - QUnit.onUnhandledRejection( event.reason ); + QUnit.onUncaughtException( event.reason ); } ); }() ); diff --git a/src/test.js b/src/test.js index 8e91f03e0..41d119d63 100644 --- a/src/test.js +++ b/src/test.js @@ -3,6 +3,7 @@ import { begin } from "./core"; import { setTimeout, clearTimeout } from "./globals"; import { emit } from "./events"; import Assert from "./assert"; +import Logger from "./logger"; import Promise from "./promise"; import config from "./core/config"; @@ -48,6 +49,20 @@ export default function Test( settings ) { this.todo = true; } + // Queuing a late test after the run has ended is not allowed. + // This was once supported for internal use by QUnit.onError(). + // Ref https://github.com/qunitjs/qunit/issues/1377 + if ( ProcessingQueue.finished ) { + + // Using this for anything other than onError(), such as testing in QUnit.done(), + // is unstable and will likely result in the added tests being ignored by CI. + // (Meaning the CI passes irregardless of the added tests). + // + // TODO: Make this an error in QUnit 3.0 + // throw new Error( "Unexpected new test after the run already ended" ); + Logger.warn( "Unexpected test after runEnd. This is unstable and will fail in QUnit 3.0." ); + return; + } if ( !this.skip && typeof this.callback !== "function" ) { const method = this.todo ? "QUnit.todo" : "QUnit.test"; throw new TypeError( `You must provide a callback to ${method}("${this.testName}")` ); @@ -452,11 +467,6 @@ Test.prototype = { this.previousFailure = !!previousFailCount; ProcessingQueue.add( runTest, prioritize, config.seed ); - - // If the queue has already finished, we manually process the new test - if ( ProcessingQueue.finished ) { - ProcessingQueue.advance(); - } }, pushResult: function( resultInfo ) { diff --git a/test/cli/fixtures/expected/tap-outputs.js b/test/cli/fixtures/expected/tap-outputs.js index db4338fb2..a78916cc8 100644 --- a/test/cli/fixtures/expected/tap-outputs.js +++ b/test/cli/fixtures/expected/tap-outputs.js @@ -113,29 +113,28 @@ Last test to run (hanging) has an async hold. Ensure all assert.async() callback `TAP version 13 not ok 1 Unhandled Rejections > test passes just fine, but has a rejected promise --- - message: Error thrown in non-returned promise! + message: global failure: Error: Error thrown in non-returned promise! severity: failed - actual : { - "message": "Error thrown in non-returned promise!", - "stack": "Error: Error thrown in non-returned promise!\\n at /some/path/wherever/unhandled-rejection.js:13:11" -} + actual : undefined expected: undefined stack: | Error: Error thrown in non-returned promise! - at /some/path/wherever/unhandled-rejection.js:13:11 + at /qunit/test/cli/fixtures/unhandled-rejection.js:10:10 + at internal ... not ok 2 global failure --- - message: outside of a test context + message: Error: outside of a test context severity: failed - actual : { - "message": "outside of a test context", - "stack": "Error: outside of a test context\\n at Object. (/some/path/wherever/unhandled-rejection.js:20:18)" -} + actual : undefined expected: undefined stack: | Error: outside of a test context - at Object. (/some/path/wherever/unhandled-rejection.js:20:18) + at Object. (/qunit/test/cli/fixtures/unhandled-rejection.js:17:18) + at processModule (/qunit/qunit/qunit.js) + at Object.module$1 [as module] (/qunit/qunit/qunit.js) + at Object. (/qunit/test/cli/fixtures/unhandled-rejection.js:3:7) + at internal ... 1..2 # pass 0 @@ -148,9 +147,9 @@ not ok 2 global failure `TAP version 13 not ok 1 global failure --- - message: No tests were run. + message: Error: No tests were run. severity: failed - actual : {} + actual : undefined expected: undefined stack: | Error: No tests were run. @@ -242,9 +241,9 @@ ok 1 Zero assertions > has a test `TAP version 13 not ok 1 global failure --- - message: "No tests matched the filter \\"no matches\\"." + message: "Error: No tests matched the filter \\"no matches\\"." severity: failed - actual : {} + actual : undefined expected: undefined stack: | Error: No tests matched the filter "no matches". diff --git a/test/cli/fixtures/unhandled-rejection.js b/test/cli/fixtures/unhandled-rejection.js index 1c41c916a..006117ca3 100644 --- a/test/cli/fixtures/unhandled-rejection.js +++ b/test/cli/fixtures/unhandled-rejection.js @@ -7,25 +7,12 @@ QUnit.module( "Unhandled Rejections", function() { 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` - }; + throw new Error( "Error thrown in non-returned promise!" ); } ); // 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. (/some/path/wherever/unhandled-rejection.js:20:18)` - } ); + Promise.reject( new Error( "outside of a test context" ) ); } ); diff --git a/test/cli/main.js b/test/cli/main.js index aa43051d5..3b9728fb8 100644 --- a/test/cli/main.js +++ b/test/cli/main.js @@ -71,7 +71,8 @@ QUnit.module( "CLI Main", () => { try { await execute( "qunit syntax-error/test.js" ); } catch ( e ) { - assert.true( e.stdout.includes( "not ok 1 syntax-error/test.js > Failed to load the test file with error:" ) ); + assert.true( e.stdout.includes( "not ok 1 global failure" ) ); + assert.true( e.stdout.includes( "Failed to load file syntax-error/test.js" ) ); assert.true( e.stdout.includes( "ReferenceError: varIsNotDefined is not defined" ) ); assert.equal( e.code, 1 ); } @@ -534,11 +535,11 @@ CALLBACK: done`; } catch ( e ) { assert.equal( e.stdout, expectedOutput[ command ] ); - // These are both undesirable, but at least confirm what the current state is. - // TDD should break these and update when possible. - assert.true( e.stderr.includes( "TypeError: Cannot read property 'length' of undefined" ), e.stderr ); - - // e.code should be 1, but is sometimes 0, 1, or 7 in different envs + assert.pushResult( { + result: e.stderr.includes( "Error: `assert.async` callback from test \"times out before scheduled done is called\" called after tests finished." ), + actual: e.stderr + } ); + assert.equal( e.code, 1 ); } } ); diff --git a/test/main/assert.js b/test/main/assert.js index 935826769..afab117de 100644 --- a/test/main/assert.js +++ b/test/main/assert.js @@ -296,6 +296,17 @@ QUnit.test( "throws", function( assert ) { "thrown object matches formatted error message" ); + assert.throws( + function() { + throw { + name: true, + message: "some message" + }; + }, + /^true: some message$/, + "formatted string for Error object with non-string name property" + ); + assert.throws( function() { throw {}; diff --git a/test/onerror/inside-test.js b/test/onerror/inside-test.js index 7df93e590..a74eb9bf9 100644 --- a/test/onerror/inside-test.js +++ b/test/onerror/inside-test.js @@ -1,41 +1,51 @@ QUnit.module( "QUnit.onError", function() { QUnit.test( "call pushFailure when inside a test", function( assert ) { - assert.expect( 3 ); + assert.expect( 2 ); - assert.test.pushFailure = function( message, source ) { - assert.strictEqual( message, "Error message", "Message is correct" ); - assert.strictEqual( source, "filePath.js:1", "Source is correct" ); + var original = assert.pushResult; + var pushed = null; + assert.pushResult = function( result ) { + pushed = result; + assert.pushResult = original; }; - var result = QUnit.onError( { + var suppressed = QUnit.onError( { message: "Error message", fileName: "filePath.js", lineNumber: 1 } ); - assert.strictEqual( result, false, "onError should allow other error handlers to run" ); + assert.strictEqual( suppressed, false, "onError should allow other error handlers to run" ); + assert.propEqual( pushed, { + result: false, + message: "global failure: Error: Error message", + source: "filePath.js:1" + }, "pushed result" ); } ); QUnit.test( "use stacktrace argument", function( assert ) { - assert.expect( 3 ); - - assert.test.pushFailure = function( message, source ) { - assert.strictEqual( message, "Error message", "Message is correct" ); - assert.strictEqual( - source, - "DummyError\nfilePath.js:1 foo()\nfilePath.js:2 bar()", - "Source is correct" - ); + assert.expect( 2 ); + + var original = assert.pushResult; + var pushed = null; + assert.pushResult = function( result ) { + pushed = result; + assert.pushResult = original; }; - var result = QUnit.onError( { + var suppressed = QUnit.onError( { message: "Error message", fileName: "filePath.js", lineNumber: 1, stacktrace: "DummyError\nfilePath.js:1 foo()\nfilePath.js:2 bar()" } ); - assert.strictEqual( result, false, "onError should allow other error handlers to run" ); + assert.strictEqual( suppressed, false, "onError should allow other error handlers to run" ); + assert.propEqual( pushed, { + result: false, + message: "global failure: Error: Error message", + source: "DummyError\nfilePath.js:1 foo()\nfilePath.js:2 bar()" + }, "pushed result" ); } ); diff --git a/test/onerror/outside-test.js b/test/onerror/outside-test.js index cf9fe33ce..b1f1c85a0 100644 --- a/test/onerror/outside-test.js +++ b/test/onerror/outside-test.js @@ -1,39 +1,47 @@ QUnit.module( "pushFailure outside a test", function( hooks ) { var originalPushResult; + var pushedName = null; + var pushedResult = null; - hooks.beforeEach( function() { + hooks.before( function( assert ) { - // Duck-punch pushResult so we can check test name and assert args. - originalPushResult = QUnit.config.current.pushResult; + // Duck-punch pushResult to capture the first test assertion. + originalPushResult = assert.pushResult; + assert.pushResult = function( resultInfo ) { + pushedName = assert.test.testName; + pushedResult = resultInfo; - QUnit.config.current.pushResult = function( resultInfo ) { + // Restore pushResult to not affect our hooks.after() assertion and dummy test. + assert.pushResult = originalPushResult; - // Restore pushResult for this assert object, to allow following assertions. - this.pushResult = originalPushResult; - - this.assert.strictEqual( - this.testName, - "global failure", "new test implicitly created and appropriately named" - ); - - this.assert.deepEqual( resultInfo, { - message: "Error message", - source: "filePath.js:1", - result: false, - actual: "actual" - }, "assert.pushResult arguments" ); + // Replace with dummy to avoid "zero assertions" error. + originalPushResult( { result: true, message: "dummy" } ); }; - } ); - hooks.afterEach( function() { - QUnit.config.current.pushResult = originalPushResult; + hooks.after( function( assert ) { + assert.strictEqual( + pushedName, + "global failure", + "new test implicitly created and appropriately named" + ); + assert.propEqual( pushedResult, { + result: false, + message: "Error: Error message", + source: "filePath.js:1" + }, "pushed result" ); } ); - // Actual test, outside QUnit.test context. + // This should generate a new test, since we're outside a QUnit.test context. QUnit.onError( { message: "Error message", fileName: "filePath.js", lineNumber: 1 - }, "actual" ); + } ); + + // This dummy test ensures hooks.after() will run even if QUnit.onError() + // failed to create the expected (failing) test. + QUnit.test( "dummy", function( assert ) { + assert.true( true ); + } ); } ); diff --git a/test/reporter-html/unhandled-rejection.js b/test/reporter-html/unhandled-rejection.js index 7cdeac2cc..d5839c47a 100644 --- a/test/reporter-html/unhandled-rejection.js +++ b/test/reporter-html/unhandled-rejection.js @@ -45,15 +45,9 @@ if ( HAS_UNHANDLED_REJECTION_HANDLER ) { this.deepEqual( resultInfo, { - message: "Error message", + message: "Error: Error message", source: "filePath.js:1", - result: false, - actual: { - message: "Error message", - fileName: "filePath.js", - lineNumber: 1, - stack: "filePath.js:1" - } + result: false }, "Expected assert.pushResult to be called with correct args" ); @@ -65,11 +59,8 @@ if ( HAS_UNHANDLED_REJECTION_HANDLER ) { } ); // Actual test, outside QUnit.test context. - QUnit.onUnhandledRejection( { - message: "Error message", - fileName: "filePath.js", - lineNumber: 1, - stack: "filePath.js:1" - } ); + var error = new Error( "Error message" ); + error.stack = "filePath.js:1"; + QUnit.onUncaughtException( error ); } ); }