From 75cc8d3e0003318cf8a8fb33cb98523f63abcfb9 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Fri, 22 Jun 2018 19:56:00 +0100 Subject: [PATCH] mediawiki.hook: Move mw.hook tests to new mediawiki.base.test.js file Follows-up 3801e54c29a. Also: * Split the test into smaller more dedicated tests. * Make minor changes so as to only assert values in the tests' outer scope. Assertions within callbacks are an anti-pattern that is fragile and can easily miss or mask problems. Using a single state observer that is modified by the callbacks makes for strict assertions with no implied or untested behaviour. Callbacks running in a different order or a different number of times now cause assertion failures - instead of causing the assertion to not be run, or to be run multiple times, which would pass. Bug: T192623 Change-Id: Ice1560b754f8df29ca583eea19f559020fafaf12 --- tests/qunit/QUnitTestResources.php | 1 + .../mediawiki/mediawiki.base.test.js | 126 ++++++++++++++++++ .../resources/mediawiki/mediawiki.test.js | 93 ------------- 3 files changed, 127 insertions(+), 93 deletions(-) create mode 100644 tests/qunit/suites/resources/mediawiki/mediawiki.base.test.js diff --git a/tests/qunit/QUnitTestResources.php b/tests/qunit/QUnitTestResources.php index 1ec6f72099..3b511c27cf 100644 --- a/tests/qunit/QUnitTestResources.php +++ b/tests/qunit/QUnitTestResources.php @@ -70,6 +70,7 @@ return [ 'tests/qunit/suites/resources/mediawiki/mediawiki.template.test.js', 'tests/qunit/suites/resources/mediawiki/mediawiki.template.mustache.test.js', 'tests/qunit/suites/resources/mediawiki/mediawiki.test.js', + 'tests/qunit/suites/resources/mediawiki/mediawiki.base.test.js', 'tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js', 'tests/qunit/suites/resources/mediawiki/mediawiki.html.test.js', 'tests/qunit/suites/resources/mediawiki/mediawiki.inspect.test.js', diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.base.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.base.test.js new file mode 100644 index 0000000000..c4159531ec --- /dev/null +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.base.test.js @@ -0,0 +1,126 @@ +( function ( mw ) { + QUnit.module( 'mediawiki.base' ); + + QUnit.test( 'mw.hook - basic', function ( assert ) { + var q = []; + mw.hook( 'test.hook.basic' ).add( function () { + q.push( 'basic' ); + } ); + + mw.hook( 'test.hook.basic' ).fire(); + assert.deepEqual( q, [ 'basic' ], 'Callback' ); + } ); + + QUnit.test( 'mw.hook - name', function ( assert ) { + var q = []; + mw.hook( 'hasOwnProperty' ).add( function () { + q.push( 'prototype' ); + } ); + + mw.hook( 'hasOwnProperty' ).fire(); + assert.deepEqual( q, [ 'prototype' ], 'Callback' ); + } ); + + QUnit.test( 'mw.hook - data', function ( assert ) { + var q; + + mw.hook( 'test.hook.data' ).add( function ( data1, data2 ) { + q = [ data1, data2 ]; + } ); + mw.hook( 'test.hook.data' ).fire( 'example', [ 'two' ] ); + + assert.deepEqual( q, + [ + 'example', + [ 'two' ] + ], + 'Data containing a string and an array' + ); + } ); + + QUnit.test( 'mw.hook - chainable', function ( assert ) { + var hook, add, fire, q = []; + + hook = mw.hook( 'test.hook.chainable' ); + assert.strictEqual( hook.add(), hook, 'hook.add is chainable' ); + assert.strictEqual( hook.remove(), hook, 'hook.remove is chainable' ); + assert.strictEqual( hook.fire(), hook, 'hook.fire is chainable' ); + + hook = mw.hook( 'test.hook.detach' ); + add = hook.add; + fire = hook.fire; + add( function ( x, y ) { + q.push( x, y ); + } ); + fire( 'x', 'y' ); + assert.deepEqual( q, [ 'x', 'y' ], 'Contextless firing with data' ); + } ); + + QUnit.test( 'mw.hook - memory before', function ( assert ) { + var q; + + q = []; + mw.hook( 'test.hook.fireBefore' ).fire().add( function () { + q.push( 'X' ); + } ); + assert.deepEqual( q, [ 'X' ], 'Remember previous firing for newly added handler' ); + + q = []; + mw.hook( 'test.hook.fireTwiceBefore' ).fire( 'Y1' ).fire( 'Y2' ).add( function ( data ) { + q.push( data ); + } ); + assert.deepEqual( q, [ 'Y2' ], 'Remember only the most recent firing' ); + } ); + + QUnit.test( 'mw.hook - memory before and after', function ( assert ) { + var q1 = [], q2 = []; + mw.hook( 'test.hook.many' ) + .add( function ( chr ) { + q1.push( chr ); + } ) + .fire( 'x' ).fire( 'y' ).fire( 'z' ) + .add( function ( chr ) { + q2.push( chr ); + } ); + + assert.deepEqual( q1, [ 'x', 'y', 'z' ], 'Multiple fires after callback addition' ); + assert.deepEqual( q2, [ 'z' ], 'Last fire applied to new handler' ); + } ); + + QUnit.test( 'mw.hook - data variadic', function ( assert ) { + var q = []; + function callback( chr ) { + q.push( chr ); + } + + mw.hook( 'test.hook.variadic' ) + .add( + callback, + callback, + function ( chr ) { + q.push( chr ); + }, + callback + ) + .fire( 'x' ) + .remove( + function () { + 'not-added'; + }, + callback + ) + .fire( 'y' ) + .remove( callback ) + .fire( 'z' ); + + assert.deepEqual( + q, + [ 'x', 'x', 'x', 'x', 'y', 'z' ], + '"add" and "remove" support variadic arguments. ' + + '"add" does not filter unique. ' + + '"remove" removes all equal by reference. ' + + '"remove" is silent if the function is not found' + ); + } ); + +}( mediaWiki ) ); diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js index 6458b17f59..83e695fcd9 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js @@ -353,97 +353,4 @@ assert.strictEqual( mw.msg( 'int-msg' ), 'Some Other Message', 'int is resolved' ); } ); - - QUnit.test( 'mw.hook', function ( assert ) { - var hook, add, fire, chars, callback; - - mw.hook( 'test.hook.unfired' ).add( function () { - assert.ok( false, 'Unfired hook' ); - } ); - - mw.hook( 'test.hook.basic' ).add( function () { - assert.ok( true, 'Basic callback' ); - } ); - mw.hook( 'test.hook.basic' ).fire(); - - mw.hook( 'hasOwnProperty' ).add( function () { - assert.ok( true, 'hook with name of predefined method' ); - } ); - mw.hook( 'hasOwnProperty' ).fire(); - - mw.hook( 'test.hook.data' ).add( function ( data1, data2 ) { - assert.strictEqual( data1, 'example', 'Fire with data (string param)' ); - assert.deepEqual( data2, [ 'two' ], 'Fire with data (array param)' ); - } ); - mw.hook( 'test.hook.data' ).fire( 'example', [ 'two' ] ); - - hook = mw.hook( 'test.hook.chainable' ); - assert.strictEqual( hook.add(), hook, 'hook.add is chainable' ); - assert.strictEqual( hook.remove(), hook, 'hook.remove is chainable' ); - assert.strictEqual( hook.fire(), hook, 'hook.fire is chainable' ); - - hook = mw.hook( 'test.hook.detach' ); - add = hook.add; - fire = hook.fire; - add( function ( x, y ) { - assert.deepEqual( [ x, y ], [ 'x', 'y' ], 'Detached (contextless) with data' ); - } ); - fire( 'x', 'y' ); - - mw.hook( 'test.hook.fireBefore' ).fire().add( function () { - assert.ok( true, 'Invoke handler right away if it was fired before' ); - } ); - - mw.hook( 'test.hook.fireTwiceBefore' ).fire().fire().add( function () { - assert.ok( true, 'Invoke handler right away if it was fired before (only last one)' ); - } ); - - chars = []; - - mw.hook( 'test.hook.many' ) - .add( function ( chr ) { - chars.push( chr ); - } ) - .fire( 'x' ).fire( 'y' ).fire( 'z' ) - .add( function ( chr ) { - assert.strictEqual( chr, 'z', 'Adding callback later invokes right away with last data' ); - } ); - - assert.deepEqual( chars, [ 'x', 'y', 'z' ], 'Multiple callbacks with multiple fires' ); - - chars = []; - callback = function ( chr ) { - chars.push( chr ); - }; - - mw.hook( 'test.hook.variadic' ) - .add( - callback, - callback, - function ( chr ) { - chars.push( chr ); - }, - callback - ) - .fire( 'x' ) - .remove( - function () { - 'not-added'; - }, - callback - ) - .fire( 'y' ) - .remove( callback ) - .fire( 'z' ); - - assert.deepEqual( - chars, - [ 'x', 'x', 'x', 'x', 'y', 'z' ], - '"add" and "remove" support variadic arguments. ' + - '"add" does not filter unique. ' + - '"remove" removes all equal by reference. ' + - '"remove" is silent if the function is not found' - ); - } ); - }( mediaWiki ) ); -- 2.20.1