User "Krinkle" posted a comment on MediaWiki.r89082. Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/89082#c17396 Commit summary:
Adding or adjusting front-end function and variable documentation per our conventions. Also did some trivial clean up, JSHint fixes and performance optimizations while at it. (per http://www.mediawiki.org/wiki/JavaScript_performance ) * Missing semicolons. * [mediawiki.js Line 540] Unreachable 'registry' after 'throw'. * [mediawiki.log.js] Checked for window.console but used console. * Added callback option to mw.util.toggleToc (forwarded to jQuery.fn.slideUp/Down) * Made mw.log argument more descriptive (logmsg instead of string) * Using deepEqual(, true) instead of ok() when asserting true. (ok() is like casting a boolean (or a plain if() statement) to verify that something is not falsy) Added missing test suites for: * mw.util.toggleToc (follow-up r88732, Dummy check was introduced in r87898) /me has been infected by Reedy and will be doing more of this tonight. Comment: I see what you mean with combining tests but I don't think it's much of an issue here. By the way, I didn't know you were referring to the <code><=</code> and <code>in</code> expressions. There are also tests for <code>mw.Map</code> / <code>"conf"</code> like <code>deepEqual( conf.exists( ... ), true, ..)</code> and <code>deepEqual( conf.set( ..., .... ), true, ..)</code>. I thought you were (also) referring to those. Because those indeed need to be made sure to 1) return boolean and, 2) return true/false per expectation. I think I've covered the range of cases fairly well here by trying to throw different things at it (see [http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/tests/qunit/suites/resources/mediawiki/mediawiki.js?view=markup&pathrev=89082#l14 here]). The <code><=</code> and <code>in</code>.. yeah, those won't need much more than simple checks. But I like to use <code>deepEqual</code> ([http://svn.wikimedia.org/viewvc/mediawiki/trunk/phase3/resources/jquery/jquery.qunit.js?revision=87848&view=markup#l432 alias] <code>same</code>), because those trigger an "Expected", "Results", "Diff" report when they don't match. ie.: :''"Expected: true"<br />"Results: undefined"'' Whereas <code>ok()</code> is really basic and just reports that it wasn't OK. deepEqual (hence it's name) aside from being a strict comparer, which is faster than loose comparison in JavaScript, in contrary to PHP) also does deep comparisons in case of objects and arrays. For example var b = { foo: 1 }; var c = { foo: 1 }; Neither loose or strict comparison will return true between these, because all objects are references in JS. deepEqual, in such case, does a resursive comparison of the members so that objects can be tested as well. Which makes it possible to easily test the object returns from functions like <code>conf.get</code> (<code>mw.Map.prototype.get</code>). That's kinda summed up (strict, expected/results output, faster than loose, recursiveness) why I tend to use deepEqual by default instead of <code>ok</code> -- Krinkle _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
