diff --git a/Gruntfile.js b/Gruntfile.js index dd45e0ee5..1befaa62d 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -90,7 +90,6 @@ module.exports = function( grunt ) { "test/reporter-urlparams-hidepassed.html", "test/moduleId.html", "test/onerror/inside-test.html", - "test/onerror/outside-test.html", "test/seed.html", "test/overload.html", "test/preconfigured.html", @@ -125,9 +124,9 @@ module.exports = function( grunt ) { "test/main/deepEqual.js", "test/main/stack.js", "test/main/utilities.js", + "test/main/onUncaughtException.js", "test/events-in-test.js", "test/onerror/inside-test.js", - "test/onerror/outside-test.js", "test/setTimeout.js", "test/node/storage-1.js", "test/node/storage-2.js", diff --git a/src/cli/run.js b/src/cli/run.js index bf0ce3dd1..d4fd930e2 100644 --- a/src/cli/run.js +++ b/src/cli/run.js @@ -95,25 +95,12 @@ async function run( args, options ) { } } - // 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", ( error, _origin ) => { QUnit.onUncaughtException( error ); - process.exitCode = 1; - uncaught = true; } ); let running = true; @@ -131,17 +118,24 @@ async function run( args, options ) { } } ); - QUnit.start(); + QUnit.on( "error", function( _error ) { + + // Set exitCode directly, to make sure it is set to fail even if "runEnd" will never be + // reached, or if "runEnd" was already fired in the past and the process crashed later. + process.exitCode = 1; + } ); QUnit.on( "runEnd", function setExitCode( data ) { running = false; - if ( data.testCounts.failed || uncaught ) { + if ( data.testCounts.failed ) { process.exitCode = 1; } else { process.exitCode = 0; } } ); + + QUnit.start(); } run.restart = function( args ) { diff --git a/src/core.js b/src/core.js index 5cf3fc72b..57209978c 100644 --- a/src/core.js +++ b/src/core.js @@ -118,7 +118,6 @@ extend( QUnit, { // Initialize the configuration options extend( config, { - stats: { all: 0, bad: 0, testCount: 0 }, started: 0, updateRate: 1000, autostart: true, diff --git a/src/core/config.js b/src/core/config.js index 7bafcb60d..19289b21a 100644 --- a/src/core/config.js +++ b/src/core/config.js @@ -11,6 +11,8 @@ const config = { // The queue of tests to run queue: [], + stats: { all: 0, bad: 0, testCount: 0 }, + // Block until document ready blocking: true, diff --git a/src/core/on-uncaught-exception.js b/src/core/on-uncaught-exception.js index 65189e58c..e32280e44 100644 --- a/src/core/on-uncaught-exception.js +++ b/src/core/on-uncaught-exception.js @@ -1,55 +1,51 @@ import config from "./config"; -import ProcessingQueue from "./processing-queue"; +import { globalSuite } from "../core"; import { sourceFromStacktrace } from "./stacktrace"; -import { extend, errorString } from "./utilities"; -import Logger from "../logger"; -import { test } from "../test"; +import { errorString } from "./utilities"; +import { emit } from "../events"; /** * 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()`. + * - If we're strictly inside a test (or one if its module hooks), the exception + * becomes a failed assertion. + * + * This has the important side-effect that uncaught exceptions (such as + * calling an undefined function) during a "todo" test do NOT result in + * a failed test run. + * + * - If we're anywhere outside a test (be it in early event callbacks, or + * internally between tests, or somewhere after "runEnd" if the process is + * still alive for some reason), then send an "error" event to the reporters. * * @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 + message: `global failure: ${errorString( error )}`, + + // We could let callers specify an offset to subtract a number of frames via + // 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 Error objects, and thus have their + // own stack trace already. + source: ( error && error.stack ) || sourceFromStacktrace( 2 ) } ); - } 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. - // - // Avoid printing "Error: foo" twice if the environment's native stack trace - // already includes that in its format. - Logger.warn( source.indexOf( source ) !== -1 ? source : `${message}\n${source}` ); + // The "error" event was added in QUnit 2.17. + // Increase "bad assertion" stats despite no longer pushing an assertion in this case. + // This ensures "runEnd" and "QUnit.done()" handlers behave as expected, since the "bad" + // count is typically how reporters decide on the boolean outcome of the test run. + globalSuite.globalFailureCount++; + config.stats.bad++; + config.stats.all++; + emit( "error", error ); } } diff --git a/src/core/onerror.js b/src/core/onerror.js index c2201e66c..e71569567 100644 --- a/src/core/onerror.js +++ b/src/core/onerror.js @@ -1,3 +1,4 @@ +import Logger from "../logger"; import config from "./config"; import onUncaughtException from "./on-uncaught-exception"; @@ -13,6 +14,7 @@ import onUncaughtException from "./on-uncaught-exception"; * to receive an "expected" error during `assert.throws()`. * * @see + * @deprecated since 2.17.0 Use QUnit.onUncaughtException instead. * @param {Object} details * @param {string} details.message * @param {string} details.fileName @@ -21,6 +23,9 @@ import onUncaughtException from "./on-uncaught-exception"; * @return {bool} True if native error reporting should be suppressed. */ export default function onWindowError( details ) { + Logger.warn( "QUnit.onError is deprecated and will be removed in QUnit 3.0." + + " Please use QUnit.onUncaughtException instead." ); + if ( config.current && config.current.ignoreGlobalErrors ) { return true; } diff --git a/src/core/processing-queue.js b/src/core/processing-queue.js index 1457d6bc9..0967d3a27 100644 --- a/src/core/processing-queue.js +++ b/src/core/processing-queue.js @@ -163,12 +163,11 @@ function done() { 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. + ProcessingQueue.finished = true; + + // We have reached the end of the processing queue and are about to emit the + // "runEnd" event after which reporters typically stop listening and exit + // the process. First, check if we need to emit one final error. if ( config.stats.testCount === 0 && config.failOnZeroTests === true ) { let error; if ( config.filter && config.filter.length ) { @@ -184,12 +183,8 @@ function done() { } onUncaughtException( error ); - advance(); - return; } - ProcessingQueue.finished = true; - emit( "runEnd", globalSuite.end( true ) ); runLoggingCallbacks( "done", { passed, diff --git a/src/events.js b/src/events.js index 3b3f57c50..db039620c 100644 --- a/src/events.js +++ b/src/events.js @@ -2,6 +2,7 @@ import { objectType, inArray } from "./core/utilities"; const LISTENERS = Object.create( null ); const SUPPORTED_EVENTS = [ + "error", "runStart", "suiteStart", "testStart", diff --git a/src/html-reporter/html.js b/src/html-reporter/html.js index a79ae0cd9..19d0562ec 100644 --- a/src/html-reporter/html.js +++ b/src/html-reporter/html.js @@ -1,11 +1,12 @@ import QUnit from "../core"; +import Logger from "../logger"; import Test from "../test"; -import { extractStacktrace } from "../core/stacktrace"; -import { now, extend } from "../core/utilities"; +import { now, extend, errorString } from "../core/utilities"; import { window, document, navigator, console } from "../globals"; import "./urlparams"; import fuzzysort from "fuzzysort"; +// TODO: Remove adhoc counting in favour of stats from QUnit.done() or "runEnd" event. const stats = { passedTests: 0, failedTests: 0, @@ -654,14 +655,18 @@ export function escapeText( s ) { title = document.createElement( "strong" ); title.innerHTML = getNameHtml( name, moduleName ); - rerunTrigger = document.createElement( "a" ); - rerunTrigger.innerHTML = "Rerun"; - rerunTrigger.href = setUrl( { testId: testId } ); - testBlock = document.createElement( "li" ); testBlock.appendChild( title ); - testBlock.appendChild( rerunTrigger ); - testBlock.id = "qunit-test-output-" + testId; + + // No ID or rerun link for "global failure" blocks + if ( testId !== undefined ) { + rerunTrigger = document.createElement( "a" ); + rerunTrigger.innerHTML = "Rerun"; + rerunTrigger.href = setUrl( { testId: testId } ); + + testBlock.id = "qunit-test-output-" + testId; + testBlock.appendChild( rerunTrigger ); + } assertList = document.createElement( "ol" ); assertList.className = "qunit-assert-list"; @@ -669,6 +674,8 @@ export function escapeText( s ) { testBlock.appendChild( assertList ); tests.appendChild( testBlock ); + + return testBlock; } // HTML Reporter initialization and load @@ -1027,6 +1034,34 @@ export function escapeText( s ) { } } ); + QUnit.on( "error", ( error ) => { + stats.failedTests++; + + const testItem = appendTest( "global failure" ); + if ( !testItem ) { + + // HTML Reporter is probably disabled or not yet initialized. + Logger.warn( "global failure" ); + Logger.warn( error ); + return; + } + + // Render similar to a failed assertion (see above QUnit.log callback) + let message = escapeText( errorString( error ) ); + message = "" + message + ""; + if ( error && error.stack ) { + message += "" + + "" + + "
Source: 
" +
+				escapeText( error.stack ) + "
"; + } + const assertList = testItem.getElementsByTagName( "ol" )[ 0 ]; + const assertLi = document.createElement( "li" ); + assertLi.className = "fail"; + assertLi.innerHTML = message; + assertList.appendChild( assertLi ); + } ); + // Avoid readyState issue with phantomjs // Ref: #818 var usingPhantom = ( function( p ) { @@ -1068,21 +1103,27 @@ export function escapeText( s ) { // Treat return value as window.onerror itself does, // Only do our handling if not suppressed. if ( ret !== true ) { - const error = { - message, - fileName, - lineNumber - }; + + // 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()`. + if ( config.current && config.current.ignoreGlobalErrors ) { + return true; + } // According to // https://blog.sentry.io/2016/01/04/client-javascript-reporting-window-onerror, // most modern browsers support an errorObj argument; use that to // get a full stack trace if it's available. - if ( errorObj && errorObj.stack ) { - error.stacktrace = extractStacktrace( errorObj, 0 ); + const error = errorObj || new Error( message ); + if ( !error.stack && fileName && lineNumber ) { + error.stack = `${fileName}:${lineNumber}`; } - - ret = QUnit.onError( error ); + QUnit.onUncaughtException( error ); } return ret; diff --git a/src/reporters/ConsoleReporter.js b/src/reporters/ConsoleReporter.js index 626f99a1b..88512d9f6 100644 --- a/src/reporters/ConsoleReporter.js +++ b/src/reporters/ConsoleReporter.js @@ -9,6 +9,7 @@ export default class ConsoleReporter { // Support IE 9: Function#bind is supported, but no console.log.bind(). this.log = options.log || Function.prototype.bind.call( console.log, console ); + runner.on( "error", this.onError.bind( this ) ); runner.on( "runStart", this.onRunStart.bind( this ) ); runner.on( "testStart", this.onTestStart.bind( this ) ); runner.on( "testEnd", this.onTestEnd.bind( this ) ); @@ -19,6 +20,10 @@ export default class ConsoleReporter { return new ConsoleReporter( runner, options ); } + onError( error ) { + this.log( "error", error ); + } + onRunStart( runStart ) { this.log( "runStart", runStart ); } diff --git a/src/reporters/TapReporter.js b/src/reporters/TapReporter.js index 3de7d4048..65c3caa6b 100644 --- a/src/reporters/TapReporter.js +++ b/src/reporters/TapReporter.js @@ -1,4 +1,5 @@ import kleur from "kleur"; +import { errorString } from "../core/utilities"; import { console } from "../globals"; const hasOwn = Object.prototype.hasOwnProperty; @@ -84,11 +85,11 @@ function prettyYamlValue( value, indent = 4 ) { // Is this a complex string? if ( value === "" || - rSpecialJson.test( value ) || - rSpecialYaml.test( value[ 0 ] ) || - rUntrimmed.test( value ) || - rNumerical.test( value ) || - rBool.test( value ) + rSpecialJson.test( value ) || + rSpecialYaml.test( value[ 0 ] ) || + rUntrimmed.test( value ) || + rNumerical.test( value ) || + rBool.test( value ) ) { if ( !/\n/.test( value ) ) { @@ -185,7 +186,10 @@ export default class TapReporter { this.log = options.log || Function.prototype.bind.call( console.log, console ); this.testCount = 0; + this.ended = false; + this.bailed = false; + runner.on( "error", this.onError.bind( this ) ); runner.on( "runStart", this.onRunStart.bind( this ) ); runner.on( "testEnd", this.onTestEnd.bind( this ) ); runner.on( "runEnd", this.onRunEnd.bind( this ) ); @@ -199,6 +203,27 @@ export default class TapReporter { this.log( "TAP version 13" ); } + onError( error ) { + if ( this.bailed ) { + return; + } + + this.bailed = true; + + // Imitate onTestEnd + // Skip this if we're past "runEnd" as it would look odd + if ( !this.ended ) { + this.testCount = this.testCount + 1; + this.log( kleur.red( `not ok ${this.testCount} global failure` ) ); + this.logError( error ); + } + + this.log( "Bail out! " + errorString( error ).split( "\n" )[ 0 ] ); + if ( this.ended ) { + this.logError( error ); + } + } + onTestEnd( test ) { this.testCount = this.testCount + 1; @@ -212,16 +237,18 @@ export default class TapReporter { this.log( kleur.cyan( `not ok ${this.testCount} # TODO ${test.fullName.join( " > " )}` ) ); - test.errors.forEach( ( error ) => this.logError( error, "todo" ) ); + test.errors.forEach( ( error ) => this.logAssertion( error, "todo" ) ); } else { this.log( kleur.red( `not ok ${this.testCount} ${test.fullName.join( " > " )}` ) ); - test.errors.forEach( ( error ) => this.logError( error ) ); + test.errors.forEach( ( error ) => this.logAssertion( error ) ); } } onRunEnd( globalSuite ) { + this.ended = true; + this.log( `1..${globalSuite.testCounts.total}` ); this.log( `# pass ${globalSuite.testCounts.passed}` ); this.log( kleur.yellow( `# skip ${globalSuite.testCounts.skipped}` ) ); @@ -229,7 +256,7 @@ export default class TapReporter { this.log( kleur.red( `# fail ${globalSuite.testCounts.failed}` ) ); } - logError( error, severity ) { + logAssertion( error, severity ) { let out = " ---"; out += `\n message: ${prettyYamlValue( error.message || "failed" )}`; out += `\n severity: ${prettyYamlValue( severity || "failed" )}`; @@ -252,4 +279,15 @@ export default class TapReporter { out += "\n ..."; this.log( out ); } + + logError( error ) { + let out = " ---"; + out += `\n message: ${prettyYamlValue( errorString( error ) )}`; + out += `\n severity: ${prettyYamlValue( "failed" )}`; + if ( error && error.stack ) { + out += `\n stack: ${prettyYamlValue( error.stack + "\n" )}`; + } + out += "\n ..."; + this.log( out ); + } } diff --git a/src/reports/suite.js b/src/reports/suite.js index fb557cb46..a355a343c 100644 --- a/src/reports/suite.js +++ b/src/reports/suite.js @@ -5,6 +5,11 @@ export default class SuiteReport { this.name = name; this.fullName = parentSuite ? parentSuite.fullName.concat( name ) : []; + // When an "error" event is emitted from onUncaughtException(), the + // "runEnd" event should report the status as failed. + // The "runEnd" event data is made by this class (as "globalSuite"). + this.globalFailureCount = 0; + this.tests = []; this.childSuites = []; @@ -71,6 +76,9 @@ export default class SuiteReport { } getTestCounts( counts = { passed: 0, failed: 0, skipped: 0, todo: 0, total: 0 } ) { + counts.failed += this.globalFailureCount; + counts.total += this.globalFailureCount; + counts = this.tests.reduce( ( counts, test ) => { if ( test.valid ) { counts[ test.getStatus() ]++; diff --git a/test/cli/ConsoleReporter.js b/test/cli/ConsoleReporter.js index c12aaed79..a7df3b143 100644 --- a/test/cli/ConsoleReporter.js +++ b/test/cli/ConsoleReporter.js @@ -34,4 +34,10 @@ QUnit.module( "ConsoleReporter", hooks => { emitter.emit( "testEnd", {} ); assert.equal( callCount, 1 ); } ); + + QUnit.test( "Event \"error\"", assert => { + emitter.emit( "error", {} ); + assert.equal( callCount, 1 ); + } ); + } ); diff --git a/test/cli/TapReporter.js b/test/cli/TapReporter.js index 06e9589d9..037269926 100644 --- a/test/cli/TapReporter.js +++ b/test/cli/TapReporter.js @@ -3,9 +3,8 @@ const { EventEmitter } = require( "events" ); function mockStack( error ) { error.stack = ` at Object. (/dev/null/test/unit/data.js:6:5) - at require (internal/modules/cjs/helpers.js:22:18) - at /dev/null/node_modules/mocha/lib/mocha.js:220:27 - at startup (internal/bootstrap/node.js:283:19)`; + at require (internal/helpers.js:22:18) + at /dev/null/src/example/foo.js:220:27`; return error; } @@ -131,23 +130,53 @@ QUnit.module( "TapReporter", hooks => { severity: failed stack: | at Object. (/dev/null/test/unit/data.js:6:5) - at require (internal/modules/cjs/helpers.js:22:18) - at /dev/null/node_modules/mocha/lib/mocha.js:220:27 - at startup (internal/bootstrap/node.js:283:19) + at require (internal/helpers.js:22:18) + at /dev/null/src/example/foo.js:220:27 ... --- message: second error severity: failed stack: | at Object. (/dev/null/test/unit/data.js:6:5) - at require (internal/modules/cjs/helpers.js:22:18) - at /dev/null/node_modules/mocha/lib/mocha.js:220:27 - at startup (internal/bootstrap/node.js:283:19) + at require (internal/helpers.js:22:18) + at /dev/null/src/example/foo.js:220:27 ... ` ); } ); + QUnit.test( "output global failure (string)", assert => { + emitter.emit( "error", "Boo" ); + + assert.strictEqual( buffer, `${kleur.red( "not ok 1 global failure" )} + --- + message: Boo + severity: failed + ... +Bail out! Boo +` + ); + } ); + + QUnit.test( "output global failure (Error)", assert => { + const err = new ReferenceError( "Boo is not defined" ); + mockStack( err ); + emitter.emit( "error", err ); + + assert.strictEqual( buffer, `${kleur.red( "not ok 1 global failure" )} + --- + message: ReferenceError: Boo is not defined + severity: failed + stack: | + at Object. (/dev/null/test/unit/data.js:6:5) + at require (internal/helpers.js:22:18) + at /dev/null/src/example/foo.js:220:27 + ... +Bail out! ReferenceError: Boo is not defined +` + ); + } ); + QUnit.test( "output expected value of Infinity", assert => { emitter.emit( "testEnd", { name: "Failing", diff --git a/test/cli/fixtures/expected/tap-outputs.js b/test/cli/fixtures/expected/tap-outputs.js index 4f3dc817b..59a704034 100644 --- a/test/cli/fixtures/expected/tap-outputs.js +++ b/test/cli/fixtures/expected/tap-outputs.js @@ -107,30 +107,29 @@ Extra reporters found among package dependencies: npm-reporter`, Last test to run (hanging) has an async hold. Ensure all assert.async() callbacks are invoked and Promises resolve. You should also set a standard timeout via QUnit.config.testTimeout.`, /* eslint-enable max-len */ "qunit unhandled-rejection.js": -`TAP version 13 -not ok 1 Unhandled Rejections > test passes just fine, but has a rejected promise +`not ok 1 global failure --- - message: global failure: Error: Error thrown in non-returned promise! + message: Error: outside of a test context severity: failed - actual : undefined - expected: undefined stack: | - Error: Error thrown in non-returned promise! - at /qunit/test/cli/fixtures/unhandled-rejection.js:10:10 + Error: outside of a test context + at /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 /qunit/test/cli/fixtures/unhandled-rejection.js:3:7 at internal ... -not ok 2 global failure +Bail out! Error: outside of a test context +TAP version 13 +not ok 2 Unhandled Rejections > test passes just fine, but has a rejected promise --- - message: Error: outside of a test context + message: global failure: Error: Error thrown in non-returned promise! severity: failed actual : undefined expected: undefined stack: | - Error: outside of a test context - at /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 /qunit/test/cli/fixtures/unhandled-rejection.js:3:7 + Error: Error thrown in non-returned promise! + at /qunit/test/cli/fixtures/unhandled-rejection.js:10:10 at internal ... 1..2 @@ -146,8 +145,6 @@ not ok 1 global failure --- message: Error: No tests were run. severity: failed - actual : undefined - expected: undefined stack: | Error: No tests were run. at done (/qunit/qunit/qunit.js) @@ -156,6 +153,7 @@ not ok 1 global failure at unblockAndAdvanceQueue (/qunit/qunit/qunit.js) at internal ... +Bail out! Error: No tests were run. 1..1 # pass 0 # skip 0 @@ -240,8 +238,6 @@ not ok 1 global failure --- message: "Error: No tests matched the filter \\"no matches\\"." severity: failed - actual : undefined - expected: undefined stack: | Error: No tests matched the filter "no matches". at done (/qunit/qunit/qunit.js) @@ -250,6 +246,7 @@ not ok 1 global failure at unblockAndAdvanceQueue (/qunit/qunit/qunit.js) at internal ... +Bail out! Error: No tests matched the filter "no matches". 1..1 # pass 0 # skip 0 @@ -381,6 +378,39 @@ not ok 1 slow # todo 0 # fail 1`, + "qunit bad-callbacks/begin-throw.js": +`TAP version 13 +not ok 1 global failure + --- + message: Error: No dice + severity: failed + stack: | + Error: No dice + at /qunit/test/cli/fixtures/bad-callbacks/begin-throw.js:2:8 + at /qunit/qunit/qunit.js + at internal + ... +Bail out! Error: No dice`, + + "qunit bad-callbacks/done-throw.js": +`TAP version 13 +ok 1 module1 > test1 +1..1 +# pass 1 +# skip 0 +# todo 0 +# fail 0 +Bail out! Error: No dice + --- + message: Error: No dice + severity: failed + stack: | + Error: No dice + at /qunit/test/cli/fixtures/bad-callbacks/done-throw.js:2:8 + at /qunit/qunit/qunit.js + at internal + ...`, + "qunit done-after-timeout.js": `TAP version 13 not ok 1 times out before scheduled done is called @@ -396,7 +426,16 @@ not ok 1 times out before scheduled done is called # pass 0 # skip 0 # todo 0 -# fail 1`, +# fail 1 +Bail out! Error: \`assert.async\` callback from test "times out before scheduled done is called" called after tests finished. + --- + message: "Error: \`assert.async\` callback from test \\"times out before scheduled done is called\\" called after tests finished." + severity: failed + stack: | + Error: \`assert.async\` callback from test "times out before scheduled done is called" called after tests finished. + at Timeout.done [as _onTimeout] (/qunit/qunit/qunit.js) + at internal + ...`, "qunit assert-expect/no-tests.js": `TAP version 13 diff --git a/test/cli/main.js b/test/cli/main.js index 50b1fc6c4..043759afc 100644 --- a/test/cli/main.js +++ b/test/cli/main.js @@ -177,6 +177,8 @@ QUnit.module( "CLI Main", () => { } } ); + // Regression test against "details of begin error swallowed" + // https://github.com/qunitjs/qunit/issues/1446 QUnit.test( "report failure in begin callback", async assert => { const command = "qunit bad-callbacks/begin-throw.js"; @@ -188,11 +190,7 @@ QUnit.module( "CLI Main", () => { } ); } catch ( e ) { assert.equal( e.code, 1 ); - - // FIXME: The details of this error are swallowed - // https://github.com/qunitjs/qunit/issues/1446 - assert.equal( e.stdout, "TAP version 13" ); - assert.equal( e.stderr, "Error: Process exited before tests finished running" ); + assert.equal( e.stdout, expectedOutput[ command ] ); } } ); @@ -207,17 +205,7 @@ QUnit.module( "CLI Main", () => { } ); } catch ( e ) { assert.equal( e.code, 1 ); - assert.equal( e.stdout, `TAP version 13 -ok 1 module1 > test1 -1..1 -# pass 1 -# skip 0 -# todo 0 -# fail 0` ); - assert.equal( e.stderr, `Error: No dice - at /qunit/test/cli/fixtures/bad-callbacks/done-throw.js:2:8 - at /qunit/qunit/qunit.js - at internal` ); + assert.equal( e.stdout, expectedOutput[ command ] ); } } ); @@ -614,13 +602,8 @@ CALLBACK: done`; try { await execute( command ); } catch ( e ) { - assert.equal( e.stdout, expectedOutput[ command ] ); - - 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 ); + assert.equal( e.stdout, expectedOutput[ command ] ); } } ); diff --git a/test/index.html b/test/index.html index 3bc0aaf61..beaf63b4d 100644 --- a/test/index.html +++ b/test/index.html @@ -18,12 +18,12 @@ + -
diff --git a/test/main/onUncaughtException.js b/test/main/onUncaughtException.js new file mode 100644 index 000000000..e20b1c4df --- /dev/null +++ b/test/main/onUncaughtException.js @@ -0,0 +1,31 @@ +QUnit.module( "QUnit.onUncaughtException", function() { + QUnit.test( "inside a test", function( assert ) { + assert.expect( 1 ); + + var original = assert.pushResult; + var pushed = null; + assert.pushResult = function( result ) { + pushed = result; + }; + + var error = new TypeError( "Message here" ); + error.stack = "filePath.js:1"; + QUnit.onUncaughtException( error ); + + assert.pushResult = original; + + assert.propEqual( pushed, { + result: false, + message: "global failure: TypeError: Message here", + source: "filePath.js:1" + }, "pushed result" ); + } ); + + // The "outside a test" scenario is not covered by explicitly calling onUncaughtException(). + // Instead, this is covered by the more standalone CLI test cases. + // + // It didn't seem worth the mess to test this since all onUncaughtException() does is + // increment numbers and call emit(). We'd have to both observe those happening in a + // private space, and then subsequently reverse the side-effects, in order to still + // have this test "pass". +} ); diff --git a/test/mozjs.js b/test/mozjs.js index 0313a297f..71a6462c8 100644 --- a/test/mozjs.js +++ b/test/mozjs.js @@ -49,5 +49,6 @@ loadRelativeToScript( "../test/main/dump.js" ); loadRelativeToScript( "../test/main/deepEqual.js" ); loadRelativeToScript( "../test/main/stack.js" ); loadRelativeToScript( "../test/main/utilities.js" ); +loadRelativeToScript( "../test/main/onUncaughtException.js" ); QUnit.start(); diff --git a/test/onerror/inside-test.js b/test/onerror/inside-test.js index a74eb9bf9..d1b6544ef 100644 --- a/test/onerror/inside-test.js +++ b/test/onerror/inside-test.js @@ -1,12 +1,11 @@ QUnit.module( "QUnit.onError", function() { - QUnit.test( "call pushFailure when inside a test", function( assert ) { + QUnit.test( "inside a test", function( assert ) { assert.expect( 2 ); var original = assert.pushResult; var pushed = null; assert.pushResult = function( result ) { pushed = result; - assert.pushResult = original; }; var suppressed = QUnit.onError( { @@ -15,6 +14,8 @@ QUnit.module( "QUnit.onError", function() { lineNumber: 1 } ); + assert.pushResult = original; + assert.strictEqual( suppressed, false, "onError should allow other error handlers to run" ); assert.propEqual( pushed, { result: false, @@ -50,20 +51,24 @@ QUnit.module( "QUnit.onError", function() { QUnit.test( "ignore failure when ignoreGlobalErrors is enabled", function( assert ) { - assert.expect( 1 ); + assert.expect( 2 ); - assert.test.pushFailure = function() { - assert.true( false, "No error should be pushed" ); + var original = assert.pushResult; + var pushed = null; + assert.pushResult = function( result ) { + pushed = result; }; - assert.test.ignoreGlobalErrors = true; - var result = QUnit.onError( { + var suppressed = QUnit.onError( { message: "Error message", fileName: "filePath.js", lineNumber: 1 } ); - assert.strictEqual( result, true, "onError should not allow other error handlers to run" ); + assert.pushResult = original; + + assert.strictEqual( pushed, null, "No error should be pushed" ); + assert.strictEqual( suppressed, true, "onError should not allow other error handlers to run" ); } ); } ); diff --git a/test/onerror/outside-test.html b/test/onerror/outside-test.html deleted file mode 100644 index ecf09dbda..000000000 --- a/test/onerror/outside-test.html +++ /dev/null @@ -1,13 +0,0 @@ - - - - - QUnit onerror tests - - - - - -
- - diff --git a/test/onerror/outside-test.js b/test/onerror/outside-test.js deleted file mode 100644 index b1f1c85a0..000000000 --- a/test/onerror/outside-test.js +++ /dev/null @@ -1,47 +0,0 @@ -QUnit.module( "pushFailure outside a test", function( hooks ) { - var originalPushResult; - var pushedName = null; - var pushedResult = null; - - hooks.before( function( assert ) { - - // Duck-punch pushResult to capture the first test assertion. - originalPushResult = assert.pushResult; - assert.pushResult = function( resultInfo ) { - pushedName = assert.test.testName; - pushedResult = resultInfo; - - // Restore pushResult to not affect our hooks.after() assertion and dummy test. - assert.pushResult = originalPushResult; - - // Replace with dummy to avoid "zero assertions" error. - originalPushResult( { result: true, message: "dummy" } ); - }; - } ); - - 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" ); - } ); - - // This should generate a new test, since we're outside a QUnit.test context. - QUnit.onError( { - message: "Error message", - fileName: "filePath.js", - lineNumber: 1 - } ); - - // 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 d5839c47a..7eda388bb 100644 --- a/test/reporter-html/unhandled-rejection.js +++ b/test/reporter-html/unhandled-rejection.js @@ -1,5 +1,5 @@ -// Detect if the current browser supports `onunhandledrejection` (avoiding -// errors for browsers without the capability) +// Detect if the current browser supports `onunhandledrejection` +// (avoiding errors in browsers without the capability) var HAS_UNHANDLED_REJECTION_HANDLER = "onunhandledrejection" in window; if ( HAS_UNHANDLED_REJECTION_HANDLER ) { @@ -8,9 +8,9 @@ if ( HAS_UNHANDLED_REJECTION_HANDLER ) { var originalPushResult = assert.pushResult; assert.pushResult = function( resultInfo ) { - // Inverts the result so we can test failing assertions + // Invert the result so we can test failing assertions resultInfo.result = !resultInfo.result; - originalPushResult( resultInfo ); + originalPushResult.call( this, resultInfo ); }; } ); @@ -26,41 +26,4 @@ if ( HAS_UNHANDLED_REJECTION_HANDLER ) { } ); } ); - - 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: Error message", - source: "filePath.js:1", - result: false - }, - "Expected assert.pushResult to be called with correct args" - ); - }; - } ); - - hooks.afterEach( function( assert ) { - assert.pushResult = originalPushResult; - } ); - - // Actual test, outside QUnit.test context. - var error = new Error( "Error message" ); - error.stack = "filePath.js:1"; - QUnit.onUncaughtException( error ); - } ); } diff --git a/test/reporter-html/window-onerror-preexisting-handler.js b/test/reporter-html/window-onerror-preexisting-handler.js index 5a8c19ec1..49e88d9be 100644 --- a/test/reporter-html/window-onerror-preexisting-handler.js +++ b/test/reporter-html/window-onerror-preexisting-handler.js @@ -2,50 +2,50 @@ /* exported onerrorReturnValue */ QUnit.module( "window.onerror (with preexisting handler)", function( hooks ) { - var originalQUnitOnError; + var originalOnUncaught; hooks.beforeEach( function() { onerrorReturnValue = null; onerrorCallingContext = null; - originalQUnitOnError = QUnit.onError; + originalOnUncaught = QUnit.onUncaughtException; } ); hooks.afterEach( function() { - QUnit.onError = originalQUnitOnError; + QUnit.onUncaughtException = originalOnUncaught; } ); - QUnit.test( "call QUnit.onError if handler returns false", function( assert ) { + QUnit.test( "call QUnit.onUncaughtException if handler returns false", function( assert ) { assert.expect( 1 ); onerrorReturnValue = false; - QUnit.onError = function() { - assert.true( true, "QUnit.onError was called" ); + QUnit.onUncaughtException = function() { + assert.true( true, "QUnit.onUncaughtException was called" ); }; window.onerror( "An error message", "filename.js", 1 ); } ); - QUnit.test( "call QUnit.onError if handler returns void 0", function( assert ) { + QUnit.test( "call QUnit.onUncaughtException if handler returns void 0", function( assert ) { assert.expect( 1 ); onerrorReturnValue = void 0; - QUnit.onError = function() { - assert.true( true, "QUnit.onError was called" ); + QUnit.onUncaughtException = function() { + assert.true( true, "QUnit.onUncaughtException was called" ); }; window.onerror( "An error message", "filename.js", 1 ); } ); - QUnit.test( "call QUnit.onError if handler returns truthy", function( assert ) { + QUnit.test( "call QUnit.onUncaughtException if handler returns truthy", function( assert ) { assert.expect( 1 ); onerrorReturnValue = "truthy value"; - QUnit.onError = function() { - assert.true( true, "QUnit.onError was called" ); + QUnit.onUncaughtException = function() { + assert.true( true, "QUnit.onUncaughtException was called" ); }; window.onerror( "An error message", "filename.js", 1 ); @@ -56,8 +56,8 @@ QUnit.module( "window.onerror (with preexisting handler)", function( hooks ) { onerrorReturnValue = true; - QUnit.onError = function() { - assert.true( false, "QUnit.onError should not have been called" ); + QUnit.onUncaughtException = function() { + assert.true( false, "QUnit.onUncaughtException should not have been called" ); }; var result = window.onerror( "An error message", "filename.js", 1 ); @@ -68,24 +68,22 @@ QUnit.module( "window.onerror (with preexisting handler)", function( hooks ) { QUnit.test( "window.onerror handler is called on window", function( assert ) { assert.expect( 1 ); - QUnit.onError = function() {}; + QUnit.onUncaughtException = function() {}; window.onerror( "An error message", "filename.js", 1 ); assert.strictEqual( onerrorCallingContext, window, "Handler called with correct context" ); } ); - QUnit.test( "forward return value of QUnit.error", function( assert ) { + QUnit.test( "forward return value of prexisting onerror", function( assert ) { assert.expect( 1 ); - var expected = {}; + onerrorReturnValue = {}; - QUnit.onError = function() { - return expected; - }; + QUnit.onUncaughtException = function() {}; var result = window.onerror( "An error message", "filename.js", 1 ); - assert.strictEqual( result, expected, "QUnit.onError return value was returned" ); + assert.strictEqual( result, onerrorReturnValue, "return value" ); } ); } ); diff --git a/test/reporter-html/window-onerror.js b/test/reporter-html/window-onerror.js index 65969278d..77dd5f918 100644 --- a/test/reporter-html/window-onerror.js +++ b/test/reporter-html/window-onerror.js @@ -1,19 +1,19 @@ QUnit.module( "window.onerror (no preexisting handler)", function( hooks ) { - var originalQUnitOnError; + var originalOnUncaught; hooks.beforeEach( function() { - originalQUnitOnError = QUnit.onError; + originalOnUncaught = QUnit.onUncaughtException; } ); hooks.afterEach( function() { - QUnit.onError = originalQUnitOnError; + QUnit.onUncaughtException = originalOnUncaught; } ); - QUnit.test( "call QUnit.onError", function( assert ) { + QUnit.test( "call QUnit.onUncaughtException", function( assert ) { assert.expect( 1 ); - QUnit.onError = function() { - assert.true( true, "QUnit.onError was called" ); + QUnit.onUncaughtException = function() { + assert.true( true, "called" ); }; window.onerror( "An error message", "filename.js", 1 ); @@ -26,25 +26,10 @@ QUnit.module( "window.onerror (no preexisting handler)", function( hooks ) { stack: "dummy.js:1 top()\ndummy.js:2 middle()\ndummy.js:3 bottom()" }; - QUnit.onError = function( error ) { - assert.equal( error.stacktrace, errorObj.stack, "QUnit.onError was called" ); + QUnit.onUncaughtException = function( error ) { + assert.equal( error.stack, errorObj.stack, "stack" ); }; window.onerror( "An error message", "filename.js", 1, 1, errorObj ); } ); - - - QUnit.test( "forward return value of QUnit.error", function( assert ) { - assert.expect( 1 ); - - var expected = {}; - - QUnit.onError = function() { - return expected; - }; - - var result = window.onerror( "An error message", "filename.js", 1 ); - - assert.strictEqual( result, expected, "QUnit.onError return value was returned" ); - } ); } );