qunit.completenessTest: Unbreak regular functions with static methods

While it already skipped constructor functions during the injection
phase (and thus didn't affect their properties), regular functions
can have properties, too.

Due to the QUnit CompletenessTest injecting a spy in mw.log,
things like mw.log.warn became undefined and causing exceptions
to be thrown sometimes.

Fix breaking of mw.log methods (and potentially other things) by
giving letting the spy function object inherit from the original
function object.

Also reduced and simplified logic further to accomodate this new
approach.

* Give injectCheck obj/key instead of masterValue/currPath.
* Pass around obj/key instead of value/key.
* Remove 'action' parameter, only ACTION_INJECT was ever used.
* Remove logic for 'prototype', this is now included in the type function/object case.
* Remove logic for 'prototype.constructor', this exclusion works naturally
  due to constructor functions being filtered out.

Change-Id: If16db9337a99865ada0fe1c482dd62a572239030

diff --git a/resources/src/jquery/jquery.qunit.completenessTest.js b/resources/src/jquery/jquery.qunit.completenessTest.js
index d6dfedd..77e38d3 100644 (file)
--- a/resources/src/jquery/jquery.qunit.completenessTest.js
+++ b/resources/src/jquery/jquery.qunit.completenessTest.js
@@ -81,8 +81,8 @@
 
                // Bind begin and end to QUnit.
                QUnit.begin( function () {
-                       that.walkTheObject( null, masterVariable, masterVariable, [], CompletenessTest.ACTION_INJECT );
-                       log( 'CompletenessTest/walkTheObject/ACTION_INJECT', that );
+                       that.walkTheObject( masterVariable, null, masterVariable, [] );
+                       log( 'CompletenessTest/walkTheObject', that );
                });
 
                QUnit.done( function () {
@@ -168,10 +168,6 @@
                return this;
        }
 
-       /* Static members */
-       CompletenessTest.ACTION_INJECT = 500;
-       CompletenessTest.ACTION_CHECK = 501;
-
        /* Public methods */
        CompletenessTest.fn = CompletenessTest.prototype = {
 
@@ -189,20 +185,24 @@
                 *  Initially this is the same as currVar.
                 * @param parentPathArray {Array} Array of names that indicate our breadcrumb path starting at
                 *  masterVariable. Not including currName.
-                * @param action {Number} What is this function supposed to do (ACTION_INJECT or ACTION_CHECK)
                 */
-               walkTheObject: function ( currName, currVar, masterVariable, parentPathArray, action ) {
-                       var key, value, currPathArray,
-                               type = util.type( currVar ),
-                               that = this;
+               walkTheObject: function ( currObj, currName, masterVariable, parentPathArray ) {
+                       var key, currVal, type,
+                               ct = this,
+                               currPathArray = parentPathArray;
 
-                       currPathArray = parentPathArray;
                        if ( currName ) {
                                currPathArray.push( currName );
+                               currVal = currObj[currName];
+                       } else {
+                               currName = '(root)';
+                               currVal = currObj;
                        }
 
+                       type = util.type( currVal );
+
                        // Hard ignores
-                       if ( this.ignoreFn( currVar, that, currPathArray ) ) {
+                       if ( this.ignoreFn( currVal, this, currPathArray ) ) {
                                return null;
                        }
 
@@ -215,45 +215,22 @@
 
                        // Functions
                        if ( type === 'function' ) {
-
-                               if ( !currVar.prototype || util.isEmptyObject( currVar.prototype ) ) {
-
-                                       if ( action === CompletenessTest.ACTION_INJECT ) {
-
-                                               that.injectionTracker[ currPathArray.join( '.' ) ] = true;
-                                               that.injectCheck( masterVariable, currPathArray, function () {
-                                                       that.methodCallTracker[ currPathArray.join( '.' ) ] = true;
-                                               } );
-                                       }
-
-                               // We don't support checking object constructors yet...
-                               // ...we can check the prototypes fine, though.
-                               } else {
-                                       if ( action === CompletenessTest.ACTION_INJECT ) {
-
-                                               for ( key in currVar.prototype ) {
-                                                       if ( hasOwn.call( currVar.prototype, key ) ) {
-                                                               value = currVar.prototype[key];
-                                                               if ( key === 'constructor' ) {
-                                                                       continue;
-                                                               }
-
-                                                               that.walkTheObject( key, value, masterVariable, currPathArray.concat( 'prototype' ), action );
-                                                       }
-                                               }
-
-                                       }
+                               // Don't put a spy in constructor functions as it messes with
+                               // instanceof etc.
+                               if ( !currVal.prototype || util.isEmptyObject( currVal.prototype ) ) {
+                                       this.injectionTracker[ currPathArray.join( '.' ) ] = true;
+                                       this.injectCheck( currObj, currName, function () {
+                                               ct.methodCallTracker[ currPathArray.join( '.' ) ] = true;
+                                       } );
                                }
-
                        }
 
                        // Recursively. After all, this is the *completeness* test
-                       if ( type === 'function' || type === 'object' ) {
-                               for ( key in currVar ) {
-                                       if ( hasOwn.call( currVar, key ) ) {
-                                               value = currVar[key];
-
-                                               that.walkTheObject( key, value, masterVariable, currPathArray.slice(), action );
+                       // This also traverses static properties and the prototype of a constructor
+                       if ( type === 'object' || type === 'function' ) {
+                               for ( key in currVal ) {
+                                       if ( hasOwn.call( currVal, key ) ) {
+                                               this.walkTheObject( currVal, key, masterVariable, currPathArray.slice() );
                                        }
                                }
                        }
@@ -294,27 +271,23 @@
                 * @param objectPathArray {Array}
                 * @param injectFn {Function}
                 */
-               injectCheck: function ( masterVariable, objectPathArray, injectFn ) {
-                       var i, len, prev, memberName, lastMember,
-                               curr = masterVariable;
-
-                       // Get the object in question through the path from the master variable,
-                       // We can't pass the value directly because we need to re-define the object
-                       // member and keep references to the parent object, member name and member
-                       // value at all times.
-                       for ( i = 0, len = objectPathArray.length; i < len; i++ ) {
-                               memberName = objectPathArray[i];
-
-                               prev = curr;
-                               curr = prev[memberName];
-                               lastMember = memberName;
-                       }
+               injectCheck: function ( obj, key, injectFn ) {
+                       var spy,
+                               val = obj[ key ];
 
-                       // Objects are by reference, members (unless objects) are not.
-                       prev[lastMember] = function () {
+                       spy = function () {
                                injectFn();
-                               return curr.apply( this, arguments );
+                               return val.apply( this, arguments );
                        };
+
+                       // Make the spy inherit from the original so that its static methods are also
+                       // visible in the spy (e.g. when we inject a check into mw.log, mw.log.warn
+                       // must remain accessible).
+                       /*jshint proto:true */
+                       spy.__proto__ = val;
+
+                       // Objects are by reference, members (unless objects) are not.
+                       obj[ key ] = spy;
                }
        };