https://bugzilla.wikimedia.org/show_bug.cgi?id=36007
Web browser: ---
Bug #: 36007
Summary: Unit test issues
Product: MediaWiki extensions
Version: unspecified
Platform: All
OS/Version: All
Status: NEW
Severity: normal
Priority: Unprioritized
Component: MobileFrontend
AssignedTo: [email protected]
ReportedBy: [email protected]
CC: [email protected], [email protected],
[email protected]
Classification: Unclassified
Mobile Platform: ---
In no particular order:
* tests/js/fixtures.js and javascripts/application.js are loaded twice.
Especially noticeable in debug mode. Please remove the redundant or use a
utility module that both depend on instead of including the same file twice in
two different modules.
* fixtures.js is executing DOM functions without waiting for document ready,
this can be a problem.
* If none of the MF tests are run (e.g. ?filter=jquery.colorUtil), then there
should be no DOM manipulations or global variables/functions defined by MF unit
test code. Schedule dom manipulation in teardown/setup functions, that way they
are automatically reset after each test (so that they don't affect each other),
and it also ensures none of them are ran if the tests aren't ran. Right now it
creates the fixtures also if none of the tests are scheduled to run.
And in debug mode it creates global variables MFE and MFET in some of the test
files, these should be local to the suite, maybe use a closure for them like
( function ($, mw, MFConf, MFE, MFET) {
/* .. */
}(jQuery, mediaWiki, mwMobileFrontendConfig, MobileFrontend,
MobileFrontendTests));
.. Or, after porting to ResourceLoader, have them all under mw.*, where mw.conf
and mw.messages are automatically reset after each test if you use (or build
on) QUnit.newMwEnvironment().
* the "qunit-fixture-x" element is non-standard pollution of the document on
Special:JavaScriptTest/qunit and isn't hidden by default. This causes it to be
unconditionally and infinitely visible in plain sight on that page when only
other test suites are ran (e.g. when setting ?filter=jquery.byteLength, the
whole MobileFrontend fixture remains in plain sight.)
QUnit has build in fixture teardown/setup after each test, using the contents
of <div id="qunit-fixture"> at time of the first test as base and resets it to
that after each test. Use the QUnit "begin" (`QUnit.begin(fn callback)`) hook
to add something to this (do not set innerHTML as that would break other test
suites fixture code, instead append to it from the test initiator via the
QUnit.begin() hook).
* This goes deeper into MobileFrontend's bigger issues, but it's using awfully
generic names and titles all in the main namespace. Such as:
- global function `wm_toggle_section`
- DIV id="logo", "nav", "sq", "search", "anchor_1", "section_1"
* Assertions: The tests for addClass and removeClass should not be using
hasClass for verification. Especially for the tests where it creates a new
element and adds one class, it is more reliable to check the raw className
property for equality instead. The hasClass assertions, similarly, should use
className directly (or just as part of the '<div ..></div>' snippet) to create
the test and then use hasClass to test it, right now it is mixing too much
leaving a fair chance of missing failures if something is wrong with either one
of them.
--
Configure bugmail: https://bugzilla.wikimedia.org/userprefs.cgi?tab=email
------- 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
