Skip to content

Commit

Permalink
Core: Error when a module callback has a promise as a return value
Browse files Browse the repository at this point in the history
  • Loading branch information
raycohen committed May 24, 2021
1 parent aae2f6d commit 5fa2774
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 24 deletions.
5 changes: 2 additions & 3 deletions src/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
20 changes: 16 additions & 4 deletions test/cli/fixtures/async-module-warning/promise-test.js
Original file line number Diff line number Diff line change
@@ -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" );
} );
24 changes: 16 additions & 8 deletions test/cli/fixtures/async-module-warning/test.js
Original file line number Diff line number Diff line change
@@ -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" );
} );
12 changes: 7 additions & 5 deletions test/cli/fixtures/expected/tap-outputs.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,18 +343,20 @@ 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`,

"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`,
Expand Down
8 changes: 4 additions & 4 deletions test/cli/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ] );
} );

Expand Down

0 comments on commit 5fa2774

Please sign in to comment.