From bd1786b1792237d4555c0749b163f5457130f0af Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Thu, 19 Jun 2014 08:06:45 +0200 Subject: [PATCH] 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 --- .../jquery/jquery.qunit.completenessTest.js | 105 +++++++----------- 1 file changed, 39 insertions(+), 66 deletions(-) diff --git a/resources/src/jquery/jquery.qunit.completenessTest.js b/resources/src/jquery/jquery.qunit.completenessTest.js index d6dfeddcf0..77e38d3c60 100644 --- 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; } }; -- 2.20.1