From 66081d66051d75f2ca10fed868eb512f5d3a6095 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Mon, 5 Jul 2021 04:03:06 +0100 Subject: [PATCH] Tests: Add tests covering callback failures Capture the status quo before changing it. Minor changes: * Switch remaining notEquals/indexOf uses to the preferred `assert.true( str.includes() )` idiom. * Fix duplicate printing of error message due to V8's `Error#stack`, as used by onUncaughtException. Ref https://github.com/qunitjs/qunit/pull/1629. * Start normalizing stderror in tests like we do with stdout. * Account for qunit.js stack frames from native Promise in V8, which doesn't include a function name or paranthesis. Ref https://github.com/qunitjs/qunit/issues/1446. Ref https://github.com/qunitjs/qunit/issues/1633. --- src/core/on-uncaught-exception.js | 5 +- .../cli/fixtures/bad-callbacks/begin-throw.js | 9 ++ test/cli/fixtures/bad-callbacks/done-throw.js | 9 ++ .../bad-callbacks/moduleDone-throw.js | 15 +++ .../fixtures/bad-callbacks/testStart-throw.js | 15 +++ test/cli/fixtures/expected/tap-outputs.js | 23 ++-- test/cli/helpers/execute.js | 11 ++ test/cli/main.js | 107 +++++++++++++++--- 8 files changed, 167 insertions(+), 27 deletions(-) create mode 100644 test/cli/fixtures/bad-callbacks/begin-throw.js create mode 100644 test/cli/fixtures/bad-callbacks/done-throw.js create mode 100644 test/cli/fixtures/bad-callbacks/moduleDone-throw.js create mode 100644 test/cli/fixtures/bad-callbacks/testStart-throw.js diff --git a/src/core/on-uncaught-exception.js b/src/core/on-uncaught-exception.js index bc1a7c5ca..65189e58c 100644 --- a/src/core/on-uncaught-exception.js +++ b/src/core/on-uncaught-exception.js @@ -47,6 +47,9 @@ export default function onUncaughtException( error ) { // 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}` ); + // + // 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}` ); } } diff --git a/test/cli/fixtures/bad-callbacks/begin-throw.js b/test/cli/fixtures/bad-callbacks/begin-throw.js new file mode 100644 index 000000000..a74df5196 --- /dev/null +++ b/test/cli/fixtures/bad-callbacks/begin-throw.js @@ -0,0 +1,9 @@ +QUnit.begin( () => { + throw new Error( "No dice" ); +} ); + +QUnit.module( "module1", () => { + QUnit.test( "test1", assert => { + assert.true( true ); + } ); +} ); diff --git a/test/cli/fixtures/bad-callbacks/done-throw.js b/test/cli/fixtures/bad-callbacks/done-throw.js new file mode 100644 index 000000000..b0af05361 --- /dev/null +++ b/test/cli/fixtures/bad-callbacks/done-throw.js @@ -0,0 +1,9 @@ +QUnit.done( () => { + throw new Error( "No dice" ); +} ); + +QUnit.module( "module1", () => { + QUnit.test( "test1", assert => { + assert.true( true ); + } ); +} ); diff --git a/test/cli/fixtures/bad-callbacks/moduleDone-throw.js b/test/cli/fixtures/bad-callbacks/moduleDone-throw.js new file mode 100644 index 000000000..3ba38a0bc --- /dev/null +++ b/test/cli/fixtures/bad-callbacks/moduleDone-throw.js @@ -0,0 +1,15 @@ +QUnit.moduleDone( details => { + throw new Error( "No dice for " + details.name ); +} ); + +QUnit.module( "module1", () => { + QUnit.test( "test1", assert => { + assert.true( true ); + } ); +} ); + +QUnit.module( "module2", () => { + QUnit.test( "test2", assert => { + assert.true( true ); + } ); +} ); diff --git a/test/cli/fixtures/bad-callbacks/testStart-throw.js b/test/cli/fixtures/bad-callbacks/testStart-throw.js new file mode 100644 index 000000000..31588164f --- /dev/null +++ b/test/cli/fixtures/bad-callbacks/testStart-throw.js @@ -0,0 +1,15 @@ +QUnit.testStart( details => { + throw new Error( "No dice for " + details.name ); +} ); + +QUnit.module( "module1", () => { + QUnit.test( "test1", assert => { + assert.true( true ); + } ); +} ); + +QUnit.module( "module2", () => { + QUnit.test( "test2", assert => { + assert.true( true ); + } ); +} ); diff --git a/test/cli/fixtures/expected/tap-outputs.js b/test/cli/fixtures/expected/tap-outputs.js index a78916cc8..4f3dc817b 100644 --- a/test/cli/fixtures/expected/tap-outputs.js +++ b/test/cli/fixtures/expected/tap-outputs.js @@ -61,7 +61,7 @@ not ok 1 Throws match > bad actual : Error: Match me with a pattern expected: "/incorrect pattern/" stack: | - at Object. (/qunit/test/cli/fixtures/fail/throws-match.js:3:10) + at /qunit/test/cli/fixtures/fail/throws-match.js:3:10 ... 1..1 # pass 0 @@ -98,16 +98,13 @@ ok 5 A-Test > derp "qunit --reporter npm-reporter": "Run ended!", "qunit --reporter does-not-exist": `No reporter found matching "does-not-exist". Built-in reporters: console, tap -Extra reporters found among package dependencies: npm-reporter -`, +Extra reporters found among package dependencies: npm-reporter`, "qunit --reporter": `Built-in reporters: console, tap -Extra reporters found among package dependencies: npm-reporter -`, +Extra reporters found among package dependencies: npm-reporter`, "qunit hanging-test": `Error: Process exited before tests finished running -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. -`, +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 @@ -130,10 +127,10 @@ not ok 2 global failure expected: undefined stack: | Error: outside of a test context - at Object. (/qunit/test/cli/fixtures/unhandled-rejection.js:17:18) + 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 Object. (/qunit/test/cli/fixtures/unhandled-rejection.js:3:7) + at /qunit/test/cli/fixtures/unhandled-rejection.js:3:7 at internal ... 1..2 @@ -175,7 +172,7 @@ not ok 2 Example > bad actual : false expected: true stack: | - at Object. (/qunit/test/cli/fixtures/sourcemap/source.js:7:14) + at /qunit/test/cli/fixtures/sourcemap/source.js:7:14 ... 1..2 # pass 1 @@ -193,7 +190,7 @@ not ok 2 Example > bad actual : false expected: true stack: | - at Object. (/qunit/test/cli/fixtures/sourcemap/sourcemap/source.js:7:10) + at /qunit/test/cli/fixtures/sourcemap/sourcemap/source.js:7:10 ... 1..2 # pass 1 @@ -299,7 +296,7 @@ not ok 1 # TODO module B > Only this module should run > a todo test actual : false expected: true stack: | - at Object. (/qunit/test/cli/fixtures/only/module.js:17:15) + at /qunit/test/cli/fixtures/only/module.js:17:15 ... ok 2 # SKIP module B > Only this module should run > implicitly skipped test ok 3 module B > Only this module should run > normal test @@ -321,7 +318,7 @@ not ok 1 # TODO module B > test B actual : false expected: true stack: | - at Object. (/qunit/test/cli/fixtures/only/module-flat.js:9:13) + at /qunit/test/cli/fixtures/only/module-flat.js:9:13 ... ok 2 # SKIP module B > test C ok 3 module B > test D diff --git a/test/cli/helpers/execute.js b/test/cli/helpers/execute.js index b2d56fa7a..af2a1b815 100644 --- a/test/cli/helpers/execute.js +++ b/test/cli/helpers/execute.js @@ -13,8 +13,18 @@ function normalize( actual ) { return actual .replace( reDir, "/qunit" ) + + // Convert "at processModule (/qunit/qunit/qunit.js:1:2)" to "at processModule (/qunit/qunit/qunit.js)" .replace( /(\/qunit\/qunit\/qunit\.js):\d+:\d+\)/g, "$1)" ) + // Convert "at /qunit/qunit/qunit.js:1:2" to "at /qunit/qunit/qunit.js" + .replace( /( {2}at \/qunit\/qunit\/qunit\.js):\d+:\d+/g, "$1" ) + + // Strip inferred names for anonymous test closures (as Node 10 did), + // to match the output of Node 12+. + // Convert "at QUnit.done (/qunit/test/foo.js:1:2)" to "at /qunit/test/foo.js:1:2" + .replace( /\b(at )\S+ \((\/qunit\/test\/[^:]+:\d+:\d+)\)/g, "$1$2" ) + // convert sourcemap'ed traces from Node 14 and earlier to the // standard format used by Node 15+. // https://github.com/nodejs/node/commit/15804e0b3f @@ -62,6 +72,7 @@ module.exports = async function execute( command, execaOptions, hook ) { return result; } catch ( e ) { e.stdout = normalize( String( e.stdout ).trimEnd() ); + e.stderr = normalize( String( e.stderr ).trimEnd() ); throw e; } }; diff --git a/test/cli/main.js b/test/cli/main.js index 3b9728fb8..50b1fc6c4 100644 --- a/test/cli/main.js +++ b/test/cli/main.js @@ -18,7 +18,7 @@ QUnit.module( "CLI Main", () => { try { await execute( "qunit does-not-exist.js" ); } catch ( e ) { - assert.equal( e.stderr.indexOf( "No files were found matching" ), 0 ); + assert.true( e.stderr.includes( "No files were found matching" ) ); } } ); @@ -155,8 +155,8 @@ QUnit.module( "CLI Main", () => { } catch ( e ) { assert.equal( e.code, 1 ); assert.equal( e.stderr, "" ); - assert.notEqual( e.stdout.indexOf( "Died on test #2 at " ), -1 ); - assert.notEqual( e.stdout.indexOf( "Error: expected error thrown in test" ), -1 ); + assert.true( e.stdout.includes( "Died on test #2 at " ) ); + assert.true( e.stdout.includes( "Error: expected error thrown in test" ) ); } } ); @@ -172,8 +172,89 @@ QUnit.module( "CLI Main", () => { } catch ( e ) { assert.equal( e.code, 1 ); assert.equal( e.stderr, "" ); - assert.notEqual( e.stdout.indexOf( "message: before failed on contains a hard error: expected error thrown in hook" ), -1 ); - assert.notEqual( e.stdout.indexOf( "Error: expected error thrown in hook" ), -1 ); + assert.true( e.stdout.includes( "message: before failed on contains a hard error: expected error thrown in hook" ) ); + assert.true( e.stdout.includes( "Error: expected error thrown in hook" ) ); + } + } ); + + QUnit.test( "report failure in begin callback", async assert => { + const command = "qunit bad-callbacks/begin-throw.js"; + + try { + const result = await execute( command ); + assert.pushResult( { + result: false, + actual: result.stdout + } ); + } 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" ); + } + } ); + + QUnit.test( "report failure in done callback", async assert => { + const command = "qunit bad-callbacks/done-throw.js"; + + try { + const result = await execute( command ); + assert.pushResult( { + result: false, + actual: result.stdout + } ); + } 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` ); + } + } ); + + QUnit.test( "report failure in moduleDone callback", async assert => { + const command = "qunit bad-callbacks/moduleDone-throw.js"; + + try { + const result = await execute( command ); + assert.pushResult( { + result: false, + actual: result.stdout + } ); + } catch ( e ) { + assert.equal( e.code, 1 ); + + // FIXME: The details of this error are swallowed + assert.equal( e.stdout, `TAP version 13 +ok 1 module1 > test1` ); + assert.equal( e.stderr, "Error: Process exited before tests finished running" ); + } + } ); + + QUnit.test( "report failure in testStart callback", async assert => { + const command = "qunit bad-callbacks/testStart-throw.js"; + + try { + const result = await execute( command ); + assert.pushResult( { + result: false, + actual: result.stdout + } ); + } catch ( e ) { + assert.equal( e.code, 1 ); + + // FIXME: The details of this error are swallowed + assert.equal( e.stdout, "TAP version 13" ); + assert.equal( e.stderr, "Error: Process exited before tests finished running" ); } } ); @@ -431,7 +512,7 @@ CALLBACK: done`; assert.pushResult( { // only in stdout due to using `console.log` in manual `unhandledRejection` handler - result: e.stdout.indexOf( "Unhandled Rejection: bad things happen sometimes" ) > -1, + result: e.stdout.includes( "Unhandled Rejection: bad things happen sometimes" ), actual: e.stdout + "\n" + e.stderr } ); } @@ -444,7 +525,7 @@ CALLBACK: done`; assert.pushResult( { // only in stdout due to using `console.log` in manual `unhandledRejection` handler - result: e.stdout.indexOf( "Unhandled Rejection: bad things happen sometimes" ) > -1, + result: e.stdout.includes( "Unhandled Rejection: bad things happen sometimes" ), actual: e.stdout + "\n" + e.stderr } ); } @@ -478,7 +559,7 @@ CALLBACK: done`; await execute( "qunit noglobals/add-global.js" ); } catch ( e ) { assert.pushResult( { - result: e.stdout.indexOf( "message: Introduced global variable(s): dummyGlobal" ) > -1, + result: e.stdout.includes( "message: Introduced global variable(s): dummyGlobal" ), actual: e.stdout + "\n" + e.stderr } ); } @@ -489,7 +570,7 @@ CALLBACK: done`; await execute( "qunit noglobals/remove-global.js" ); } catch ( e ) { assert.pushResult( { - result: e.stdout.indexOf( "message: Deleted global variable(s): dummyGlobal" ) > -1, + result: e.stdout.includes( "message: Deleted global variable(s): dummyGlobal" ), actual: e.stdout + "\n" + e.stderr } ); } @@ -519,7 +600,7 @@ CALLBACK: done`; await execute( "qunit semaphore/restart.js" ); } catch ( e ) { assert.pushResult( { - result: e.stdout.indexOf( "message: \"Tried to restart test while already started (test's semaphore was 0 already)" ) > -1, + result: e.stdout.includes( "message: \"Tried to restart test while already started (test's semaphore was 0 already)" ), actual: e.stdout + "\n" + e.stderr } ); } @@ -652,7 +733,7 @@ CALLBACK: done`; assert.equal( e.stderr, "" ); // can't match exactly due to stack frames including internal line numbers - assert.notEqual( e.stdout.indexOf( "message: Expected 2 assertions, but 1 were run" ), -1, e.stdout ); + assert.true( e.stdout.includes( "message: Expected 2 assertions, but 1 were run" ), e.stdout ); } } ); @@ -669,7 +750,7 @@ CALLBACK: done`; assert.equal( e.stderr, "" ); // can't match exactly due to stack frames including internal line numbers - assert.notEqual( e.stdout.indexOf( "Expected at least one assertion, but none were run - call expect(0) to accept zero assertions." ), -1, e.stdout ); + assert.true( e.stdout.includes( "Expected at least one assertion, but none were run - call expect(0) to accept zero assertions." ), e.stdout ); } } ); @@ -684,7 +765,7 @@ CALLBACK: done`; } catch ( e ) { assert.equal( e.code, 1 ); assert.equal( e.stderr, "" ); - assert.notEqual( e.stdout.indexOf( "message: Expected number of assertions to be defined, but expect() was not called." ), -1, e.stdout ); + assert.true( e.stdout.includes( "message: Expected number of assertions to be defined, but expect() was not called." ), e.stdout ); } } ); } );