From b868a2da807335e3a762451521c52445ebbe5930 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Sun, 10 Aug 2014 22:57:47 -0400 Subject: [PATCH] Eventable common tests for #{on,{add,remove}Listener} --- src/event/Evented.js | 43 +++++--- test/event/EventableTestConform.js | 164 +++++++++++++++-------------- 2 files changed, 117 insertions(+), 90 deletions(-) diff --git a/src/event/Evented.js b/src/event/Evented.js index cb85c1a..cb9e5f5 100644 --- a/src/event/Evented.js +++ b/src/event/Evented.js @@ -230,17 +230,9 @@ module.exports = Trait( 'Evented' ) */ on( ev, listener ) { - if ( !( typeof ev === 'string' && ev ) ) { - throw TypeError( "Missing event identifier" ); - } - else if ( !( this._events[ ev ] ) ) { - throw ReferenceError( `Cannot hook undefined event \`${ev}'` ); - } + this._argValidate( ev, listener, 'hook' ); - if ( typeof listener !== 'function' ) { - throw TypeError( "Event listener must be a function" ); - } - else if ( this.hooksEvent( ev, listener ) ) + if ( this.hooksEvent( ev, listener ) ) { throw Error( `Listener has already been bound to event \`${ev}'` @@ -252,6 +244,33 @@ module.exports = Trait( 'Evented' ) }, + /** + * Ensures that EV and LISTENER conform to Eventable + * + * @O{1} constant time + * + * @param {string} ev event id + * @param {Function} listener hook function + * + * @return {undefined} + */ + _argValidate( ev, listener, action ) + { + if ( !( typeof ev === 'string' && ev ) ) { + throw TypeError( "Missing event identifier" ); + } + else if ( !( this._events[ ev ] ) ) { + throw ReferenceError( + `Cannot ${action} undefined event \`${ev}'` + ); + } + + if ( typeof listener !== 'function' ) { + throw TypeError( "Event listener must be a function" ); + } + }, + + /** * Adds event listener, ensuring that gaps created by removing listeners * are filled if available @@ -356,9 +375,7 @@ module.exports = Trait( 'Evented' ) */ removeListener( ev, listener ) { - if ( !( this._events[ ev ] ) ) { - throw Error( `Cannot unhook undefined event \`${ev}'` ); - } + this._argValidate( ev, listener, 'unhook' ); this.unhookEvent( ev, listener ); return this; diff --git a/test/event/EventableTestConform.js b/test/event/EventableTestConform.js index c461437..b3eb44e 100644 --- a/test/event/EventableTestConform.js +++ b/test/event/EventableTestConform.js @@ -79,10 +79,90 @@ module.exports = ctor => [ 'on', 'addListener' ].forEach( x => _onTests( meta_ctor, x ) ); + + _rmTests( meta_ctor ); } ); }; +function _commonTests( ctor, method ) +{ + /** + * We must test both inputs at once since we cannot otherwise say + * with confidence which parameter is non-conforming. + */ + it( 'accepts string event id with listener function', () => + expect( () => ctor()[ method ]( _ev, _fvoid ) ) + .to.not.throw( Error ) ); + + + /** + * A TypeError should be thrown when the event id is empty. The + * rationale behind this is that the event id may be dynamically + * determined at runtime, or may otherwise be stored in a variable, + * which probably is not supposed to be empty; if it is, this could + * represent a logic error (such as an incomplete conditional or + * table lookup failure). More likely, this would result in + * `undefined`, which is covered in below tests. + * + * Implementations wishing to use an empty string to indicate "all + * events" could instead use, for example, `*` (motivated by shell + * globbing). + */ + it( 'does not accept an empty event id', () => + { + expect( () => ctor()[ method ]( '', _fvoid ) ) + .to.throw( TypeError ); + } ); + + + /** + * All event identifiers should be strings; this provides + * consistency between all implementations and helps to weed out + * runtime bugs (see the "empty event id" test for examples). + */ + it( 'does not accept non-string event ids', () => + { + _nonstr.forEach( badev => + { + expect( () => ctor()[ method ]( badev, _fvoid ) ) + .to.throw( TypeError ); + } ); + } ); + + + /** + * Same rationale as the event string argument. + */ + it( 'does not accept non-function listeners', () => + { + _nonf.forEach( badf => + { + expect( () => ctor()[ method ]( _ev, badf ) ) + .to.throw( TypeError ); + } ); + } ); + + + /** + * When hooking multiple events on a single object, it is convenient + * to be able to do so concisely without having to repeat the name + * of the object reference. + * + * Since exceptions are used to indicate error conditions, there is + * also no other useful value to return that would not break + * encapsulation. + */ + it( 'returns self for method chaining', () => + { + let inst = ctor(); + + expect( inst[ method ]( _ev, _fvoid ) ) + .to.equal( inst ); + } ); +} + + function _onTests( ctor, on ) { /** @@ -92,83 +172,13 @@ function _onTests( ctor, on ) */ describe( `#${on}`, () => { - /** - * We must test both inputs at once since we cannot otherwise say - * with confidence which parameter is non-conforming. - */ - it( 'accepts string event id with listener function', () => - { - expect( () => - { - ctor()[ on ]( _ev, _fvoid ); - } ).to.not.throw( Error ); - } ); - - - /** - * A TypeError should be thrown when the event id is empty. The - * rationale behind this is that the event id may be dynamically - * determined at runtime, or may otherwise be stored in a variable, - * which probably is not supposed to be empty; if it is, this could - * represent a logic error (such as an incomplete conditional or - * table lookup failure). More likely, this would result in - * `undefined`, which is covered in below tests. - * - * Implementations wishing to use an empty string to indicate "all - * events" could instead use, for example, `*` (motivated by shell - * globbing). - */ - it( 'does not accept an empty event id', () => - { - expect( () => ctor()[ on ]( '', _fvoid ) ) - .to.throw( TypeError ); - } ); - - - /** - * All event identifiers should be strings; this provides - * consistency between all implementations and helps to weed out - * runtime bugs (see the "empty event id" test for examples). - */ - it( 'does not accept non-string event ids', () => - { - _nonstr.forEach( badev => - { - expect( () => ctor()[ on ]( badev, _fvoid ) ) - .to.throw( TypeError ); - } ); - } ); - - - /** - * Same rationale as the event string argument. - */ - it( 'does not accept non-function listeners', () => - { - _nonf.forEach( badf => - { - expect( () => ctor()[ on ]( _ev, badf ) ) - .to.throw( TypeError ); - } ); - } ); - - - /** - * When hooking multiple events on a single object, it is convenient - * to be able to do so concisely without having to repeat the name - * of the object reference. - * - * Since exceptions are used to indicate error conditions, there is - * also no other useful value to return that would not break - * encapsulation. - */ - it( 'returns self for method chaining', () => - { - let inst = ctor(); - - expect( inst[ on ]( _ev, _fvoid ) ) - .to.equal( inst ); - } ); + _commonTests( ctor, on ); } ); } + +function _rmTests( ctor ) +{ + _commonTests( ctor, 'removeListener' ); +} +