From f3f43e34cf4ab8503b60634438347cc427002272 Mon Sep 17 00:00:00 2001 From: step2yeung Date: Wed, 26 Sep 2018 13:05:04 -0700 Subject: [PATCH] Core: handling callback promises serially --- src/core/logging.js | 18 +++++- src/core/processing-queue.js | 19 ++++--- src/test.js | 54 +++++++++--------- test/callbacks-promises.js | 103 +++++++++++++++++++++++++++-------- 4 files changed, 135 insertions(+), 59 deletions(-) diff --git a/src/core/logging.js b/src/core/logging.js index a65b2dcab..bc062e69b 100644 --- a/src/core/logging.js +++ b/src/core/logging.js @@ -35,6 +35,20 @@ export function registerLoggingCallbacks( obj ) { export function runLoggingCallbacks( key, args ) { var callbacks = config.callbacks[ key ]; - var promises = callbacks.map( callback => Promise.resolve( callback( args ) ) ); - return Promise.all( promises ); + + // Handling 'log' callbacks separately. Unlike the other callbacks, + // the log callback is not controlled by the processing queue, + // but rather used by asserts. Hence to promisfy the 'log' callback + // would mean promisfying each step of a test + if ( key === "log" ) { + callbacks.map( callback => callback( args ) ); + return; + } + + // ensure that each callback is executed serially + return callbacks.reduce( ( promiseChain, callback ) => { + return promiseChain.then( () => { + return Promise.resolve( callback( args ) ); + } ); + }, Promise.resolve( [] ) ); } diff --git a/src/core/processing-queue.js b/src/core/processing-queue.js index 70a9f6e78..657a3968e 100644 --- a/src/core/processing-queue.js +++ b/src/core/processing-queue.js @@ -33,7 +33,7 @@ const taskQueue = []; function advance() { advanceTaskQueue(); - if ( !taskQueue.length ) { + if ( !taskQueue.length && !config.blocking ) { advanceTestQueue(); } } @@ -192,18 +192,19 @@ function done() { failed: config.stats.bad, total: config.stats.all, runtime - } ); + } ).then( () => { - // Clear own storage items if all tests passed - if ( storage && config.stats.bad === 0 ) { - for ( let i = storage.length - 1; i >= 0; i-- ) { - const key = storage.key( i ); + // Clear own storage items if all tests passed + if ( storage && config.stats.bad === 0 ) { + for ( let i = storage.length - 1; i >= 0; i-- ) { + const key = storage.key( i ); - if ( key.indexOf( "qunit-test-" ) === 0 ) { - storage.removeItem( key ); + if ( key.indexOf( "qunit-test-" ) === 0 ) { + storage.removeItem( key ); + } } } - } + } ); } const ProcessingQueue = { diff --git a/src/test.js b/src/test.js index 61d3294e9..55db8444d 100644 --- a/src/test.js +++ b/src/test.js @@ -105,27 +105,29 @@ function getNotStartedModules( startModule ) { module = module.parentModule; } - return modules; + // The above push modules from the child to the parent + // return a reversed order with the top being the top most parent module + return modules.reverse(); } Test.prototype = { before: function() { - var i, startModule, - module = this.module, - promises = [], + var module = this.module, notStartedModules = getNotStartedModules( module ); - for ( i = notStartedModules.length - 1; i >= 0; i-- ) { - startModule = notStartedModules[ i ]; - startModule.stats = { all: 0, bad: 0, started: now() }; - emit( "suiteStart", startModule.suiteReport.start( true ) ); - promises.push( runLoggingCallbacks( "moduleStart", { - name: startModule.name, - tests: startModule.tests - } ) ); - } + // ensure the callbacks are executed serially for each module + var callbackPromises = notStartedModules.reduce( ( promiseChain, startModule ) => { + return promiseChain.then( () => { + startModule.stats = { all: 0, bad: 0, started: now() }; + emit( "suiteStart", startModule.suiteReport.start( true ) ); + return runLoggingCallbacks( "moduleStart", { + name: startModule.name, + tests: startModule.tests + } ); + } ); + }, Promise.resolve( [] ) ); - return Promise.all( promises ).then( () => { + return callbackPromises.then( () => { config.current = this; this.testEnvironment = extend( {}, module.testEnvironment ); @@ -137,11 +139,11 @@ Test.prototype = { module: module.name, testId: this.testId, previousFailure: this.previousFailure + } ).then( () => { + if ( !config.pollution ) { + saveGlobal(); + } } ); - } ).then( () => { - if ( !config.pollution ) { - saveGlobal(); - } } ); }, @@ -334,20 +336,21 @@ Test.prototype = { source: this.stack } ).then( function() { if ( module.testsRun === numberOfTests( module ) ) { - return logSuiteEnd( module ); - } - } ).then( function() { - if ( module.testsRun === numberOfTests( module ) ) { - const promises = []; + const completedModules = [ module ]; // Check if the parent modules, iteratively, are done. If that the case, // we emit the `suiteEnd` event and trigger `moduleDone` callback. let parent = module.parentModule; while ( parent && parent.testsRun === numberOfTests( parent ) ) { - promises.push( logSuiteEnd( parent ) ); + completedModules.push( parent ); parent = parent.parentModule; } - return Promise.all( promises ); + + return completedModules.reduce( ( promiseChain, completedModule ) => { + return promiseChain.then( () => { + return logSuiteEnd( completedModule ); + } ); + }, Promise.resolve( [] ) ); } } ).then( function() { config.current = undefined; @@ -368,7 +371,6 @@ Test.prototype = { total: module.stats.all, runtime: now() - module.stats.started } ); - } }, diff --git a/test/callbacks-promises.js b/test/callbacks-promises.js index 615fd356e..f69058d1c 100644 --- a/test/callbacks-promises.js +++ b/test/callbacks-promises.js @@ -1,5 +1,5 @@ var done = false; -var callbackCalled = {}; +var invokedHooks = []; function timeoutPromiseCallback( callback, timeout ) { return new Promise( function( resolve ) { @@ -12,35 +12,44 @@ function timeoutPromiseCallback( callback, timeout ) { QUnit.begin( function() { return timeoutPromiseCallback( function() { - callbackCalled.begin = true; + invokedHooks.push( "begin" ); }, 100 ); } ); +QUnit.begin( function() { + return timeoutPromiseCallback( function() { + + // This is the second begin callback, which should + // be executed only after the first one + if ( invokedHooks.indexOf( "begin" ) !== 0 ) { + return; + } + invokedHooks.push( "begin2" ); + }, 10 ); +} ); QUnit.moduleStart( function() { return timeoutPromiseCallback( function() { - callbackCalled.moduleStart = true; + invokedHooks.push( "moduleStart" ); }, 100 ); } ); -QUnit.testStart( function() { +QUnit.testStart( function( cb ) { return timeoutPromiseCallback( function() { - callbackCalled.testStart = true; + invokedHooks.push( "testStart - " + cb.name ); }, 100 ); } ); -QUnit.testDone( function() { +QUnit.testDone( function( cb ) { return timeoutPromiseCallback( function() { - callbackCalled.testStart = false; - callbackCalled.testDone = true; + invokedHooks.push( "testDone - " + cb.name ); }, 100 ); } ); -QUnit.moduleDone( function() { +QUnit.moduleDone( function( cb ) { return timeoutPromiseCallback( function() { - callbackCalled.moduleStart = false; - callbackCalled.moduleDone = true; + invokedHooks.push( "moduleDone - " + cb.name ); }, 100 ); } ); QUnit.done( function() { return timeoutPromiseCallback( function() { - callbackCalled.done = true; + invokedHooks.push( "done" ); }, 100 ); } ); @@ -52,19 +61,69 @@ QUnit.done( function() { done = true; QUnit.test( "verify callback order", function( assert ) { - assert.ok( callbackCalled.begin ); - assert.notOk( callbackCalled.moduleStart ); - assert.notOk( callbackCalled.testStart ); - assert.ok( callbackCalled.testDone ); - assert.ok( callbackCalled.moduleDone ); - assert.ok( callbackCalled.done ); + assert.deepEqual( invokedHooks, [ + "begin", + "begin2", + "moduleStart", + "moduleStart", + "testStart - test1", + "testDone - test1", + "moduleDone - module1 > nestedModule1", + "testStart - test2", + "testDone - test2", + "moduleStart", + "testStart - test3", + "testDone - test3", + "moduleDone - module1 > nestedModule2", + "moduleDone - module1", + "done", + "moduleStart", + "testStart - verify callback order" + ] ); } ); } ); QUnit.module( "module1", function() { - QUnit.test( "test1", function( assert ) { - assert.ok( callbackCalled.begin ); - assert.ok( callbackCalled.moduleStart ); - assert.ok( callbackCalled.testStart ); + QUnit.module( "nestedModule1", function() { + QUnit.test( "test1", function( assert ) { + assert.deepEqual( invokedHooks, [ + "begin", + "begin2", + "moduleStart", + "moduleStart", + "testStart - test1" + ] ); + } ); + } ); + + QUnit.test( "test2", function( assert ) { + assert.deepEqual( invokedHooks, [ + "begin", + "begin2", + "moduleStart", + "moduleStart", + "testStart - test1", + "testDone - test1", + "moduleDone - module1 > nestedModule1", + "testStart - test2" + ] ); + } ); + + QUnit.module( "nestedModule2", function() { + QUnit.test( "test3", function( assert ) { + assert.deepEqual( invokedHooks, [ + "begin", + "begin2", + "moduleStart", + "moduleStart", + "testStart - test1", + "testDone - test1", + "moduleDone - module1 > nestedModule1", + "testStart - test2", + "testDone - test2", + "moduleStart", + "testStart - test3" + ] ); + } ); } ); } );