Eventable common tests for #{on,{add,remove}Listener}

src/event/Evented.js

@@ -230,17 +230,9 @@ module.exports = Trait( 'Evented' )
      */
     on( ev, listener )
     {
-        if ( !( typeof ev === 'string' && ev ) ) {
+        this._argValidate( ev, listener, 'hook' );
-            throw TypeError( "Missing event identifier" );
-        }
-        else if ( !( this._events[ ev ] ) ) {
-            throw ReferenceError( `Cannot hook undefined event \`${ev}'` );
-        }
 
-        if ( typeof listener !== 'function' ) {
+        if ( this.hooksEvent( ev, listener ) )
-            throw TypeError( "Event listener must be a function" );
-        }
-        else 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 ] ) ) {
+        this._argValidate( ev, listener, 'unhook' );
-            throw Error( `Cannot unhook undefined event \`${ev}'` );
-        }
 
         this.unhookEvent( ev, listener );
         return this;

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}`, () =>
     {
-        /**
+        _commonTests( ctor, 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 );
-        } );
     } );
 }
 
+
+function _rmTests( ctor )
+{
+    _commonTests( ctor, 'removeListener' );
+}
+