From 5fa2774a5250466542041400cca883924c8b9b1c Mon Sep 17 00:00:00 2001 From: Raymond Cohen Date: Sun, 23 May 2021 20:16:32 -0400 Subject: [PATCH] Core: Error when a module callback has a promise as a return value Ref https://github.com/qunitjs/qunit/issues/1600. --- src/module.js | 5 ++-- .../async-module-warning/promise-test.js | 20 ++++++++++++---- .../cli/fixtures/async-module-warning/test.js | 24 ++++++++++++------- test/cli/fixtures/expected/tap-outputs.js | 12 ++++++---- test/cli/main.js | 8 +++---- 5 files changed, 45 insertions(+), 24 deletions(-) diff --git a/src/module.js b/src/module.js index c7d747534..c9e607123 100644 --- a/src/module.js +++ b/src/module.js @@ -86,9 +86,8 @@ function processModule( name, options, executeNow, modifiers = {} ) { const cbReturnValue = executeNow.call( module.testEnvironment, moduleFns ); if ( cbReturnValue != null && objectType( cbReturnValue.then ) === "function" ) { - Logger.warn( "Returning a promise from a module callback is not supported. " + - "Instead, use hooks for async behavior. " + - "This will become an error in QUnit 3.0." ); + throw new Error( "Returning a promise from a module callback is not supported. " + + "Instead, use hooks for async behavior." ); } moduleStack.pop(); diff --git a/test/cli/fixtures/async-module-warning/promise-test.js b/test/cli/fixtures/async-module-warning/promise-test.js index 9474de11b..37d3c10fe 100644 --- a/test/cli/fixtures/async-module-warning/promise-test.js +++ b/test/cli/fixtures/async-module-warning/promise-test.js @@ -1,6 +1,18 @@ -QUnit.module( "module manually returning a promise", function() { - QUnit.test( "has a test", function( assert ) { - assert.true( true ); +var errorFromModule; +try { + QUnit.module( "module manually returning a promise", function() { + QUnit.test( "has a test", function( assert ) { + assert.true( true ); + } ); + return Promise.resolve( 1 ); } ); - return Promise.resolve( 1 ); +} catch ( e ) { + errorFromModule = e; +} + +QUnit.test( "Correct error thrown from module function that returns a promise", function( assert ) { + assert.true( errorFromModule instanceof Error ); + assert.strictEqual( errorFromModule.message, + "Returning a promise from a module callback is not supported. " + + "Instead, use hooks for async behavior.", "Error has correct message" ); } ); diff --git a/test/cli/fixtures/async-module-warning/test.js b/test/cli/fixtures/async-module-warning/test.js index e28ae64d3..bceb59839 100644 --- a/test/cli/fixtures/async-module-warning/test.js +++ b/test/cli/fixtures/async-module-warning/test.js @@ -1,10 +1,18 @@ -// eslint-disable-next-line qunit/no-async-module-callbacks -QUnit.module( "module with async callback", async function() { - await Promise.resolve( 1 ); - - QUnit.test( "has a test", function( assert ) { - assert.true( true ); +var errorFromAsyncModule; +try { + /* eslint-disable-next-line qunit/no-async-module-callbacks */ + QUnit.module( "async module function", async function() { + QUnit.test( "has a test", function( assert ) { + assert.true( true ); + } ); } ); -} ); +} catch ( e ) { + errorFromAsyncModule = e; +} -QUnit.module( "resulting parent module" ); +QUnit.test( "Correct error thrown from async module function", function( assert ) { + assert.true( errorFromAsyncModule instanceof Error ); + assert.strictEqual( errorFromAsyncModule.message, + "Returning a promise from a module callback is not supported. " + + "Instead, use hooks for async behavior.", "Error has correct message" ); +} ); diff --git a/test/cli/fixtures/expected/tap-outputs.js b/test/cli/fixtures/expected/tap-outputs.js index 97a4024d7..838bc0d5a 100644 --- a/test/cli/fixtures/expected/tap-outputs.js +++ b/test/cli/fixtures/expected/tap-outputs.js @@ -343,9 +343,10 @@ ok 1 module providing hooks > module not providing hooks > has a test "qunit async-module-warning/test.js": `TAP version 13 -ok 1 resulting parent module > has a test -1..1 -# pass 1 +ok 1 async module function > has a test +ok 2 async module function > Correct error thrown from async module function +1..2 +# pass 2 # skip 0 # todo 0 # fail 0`, @@ -353,8 +354,9 @@ ok 1 resulting parent module > has a test "qunit async-module-warning/promise-test.js": `TAP version 13 ok 1 module manually returning a promise > has a test -1..1 -# pass 1 +ok 2 module manually returning a promise > Correct error thrown from module function that returns a promise +1..2 +# pass 2 # skip 0 # todo 0 # fail 0`, diff --git a/test/cli/main.js b/test/cli/main.js index 2c5be859c..a9ed24d42 100644 --- a/test/cli/main.js +++ b/test/cli/main.js @@ -498,21 +498,21 @@ QUnit.module( "CLI Main", () => { assert.equal( execution.stdout, expectedOutput[ command ] ); } ); - QUnit.test( "warns about unsupported async module callback", async assert => { + QUnit.test( "errors on async module callback", async assert => { const command = "qunit async-module-warning/test.js"; const execution = await execute( command ); assert.equal( execution.code, 0 ); - assert.equal( execution.stderr, "Returning a promise from a module callback is not supported. Instead, use hooks for async behavior. This will become an error in QUnit 3.0.", "The warning shows" ); + assert.equal( execution.stderr, "" ); assert.equal( execution.stdout, expectedOutput[ command ] ); } ); - QUnit.test( "warns about unsupported promise return value from module", async assert => { + QUnit.test( "errors on promise return value from module", async assert => { const command = "qunit async-module-warning/promise-test.js"; const execution = await execute( command ); assert.equal( execution.code, 0 ); - assert.equal( execution.stderr, "Returning a promise from a module callback is not supported. Instead, use hooks for async behavior. This will become an error in QUnit 3.0.", "The warning shows" ); + assert.equal( execution.stderr, "" ); assert.equal( execution.stdout, expectedOutput[ command ] ); } );