https://bugzilla.wikimedia.org/show_bug.cgi?id=49032
Krinkle <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Have Jenkins run QUnit |Jenkins: Create QUnit job |tests for GuidedTour |for GuidedTour --- Comment #1 from Krinkle <[email protected]> --- I'll enable this soon (after I verify the test suite works properly). Meanwhile a few (non-blocking) points based on a quick look at the file[1]: * setup/teardown for mw.config wgPageName is pointless (newMwEnvironment already does this for all of mw.config. If there's a problem, please file a bug and/or document the hack). * Testing through getParamValue $.cookie looks very suspicious. If it is indeed testing something, it certainly isn't a unit test for the logic of a particular method. Please revisit this and remove tests and/or refactor methods and their tests accordingly. Perhaps some inappropriately mixed app logic with UI. * Avoid use of assert.ok[2]. It is almost always wrong (that is to say, I've yet to first see production use of it in a good way). In case of the instanceof you might for example want to do a `assert.strictEqual( a.constructor, b, ..`. assert.ok does not give any information if the assertion is wrong, hard to debug. It also makes it easy to make a mistake, as my next point shows. * Broken use of assert.ok[3] (asserting that a literal function is "ok", which is pointless as functions are objects which cast to boolean true). This should probably assert the return value of invoking the function. Had this been compared the value to something (as opposed to "ok"), this bug would've been exposed immediately. * Suspicious global private static "gt = mw.guidedTour;". What does this achieve when put in the QUnit setup hook that is ran before each test? Note that the test suite itself appears to run fine so this won't block enabling of it via Jenkins. [1] https://github.com/wikimedia/mediawiki-extensions-GuidedTour/blob/5f16b1fcb7/tests/ext.guidedTour.lib.tests.js [2] http://www.mediawiki.org/wiki/Manual:JavaScript_unit_testing/QUnit_guidelines [3] https://github.com/wikimedia/mediawiki-extensions-GuidedTour/blob/5f16b1fcb7/tests/ext.guidedTour.lib.tests.js#L282-L305 -- You are receiving this mail because: You are the assignee for the bug. You are on the CC list for the bug. _______________________________________________ Wikibugs-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
