Last modified: 2012-10-13 06:10:15 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T42892, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 40892 - ResourceLoader: Missing files in a module should trigger an error
ResourceLoader: Missing files in a module should trigger an error
Status: RESOLVED WONTFIX
Product: MediaWiki
Classification: Unclassified
ResourceLoader (Other open bugs)
unspecified
All All
: Normal normal (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-09 13:21 UTC by Daniel A. R. Werner
Modified: 2012-10-13 06:10 UTC (History)
5 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Daniel A. R. Werner 2012-10-09 13:21:38 UTC
Non-existent QUnit test scripts registered to a test module via the 'ResourceLoaderTestModules' hook will prevent all following scripts from being executed. No error or failure shown.

There should be a error displayed instead. Probably best if the QUnit view would turn red and display an appropriate message at the point where the missing script should have been executed.
Comment 1 Krinkle 2012-10-09 16:13:44 UTC
This is outside the scope of QUnit because QUnit can't do anything when it isn't given anything. These modules are registered with ResourceLoader and that's where the error handling is.

If a module (test module, regular module, makes no difference) is invalidly defined, it won't be delivered to the client. There is nothing it can do.

In the web browser in the debugger of your choice you will find a stack trace in a /* comment */ on top of all subsequent load.php requests with information about this. Though it is probably easier to catch in your server-side error log, which you should activate[1] when developing locally. MediaWiki will write all the info you need to the configured error log.

[1] https://www.google.com/search?q=mediawiki+debug&btnI=ifl
By default any serious errors will be logged to the php.ini or Apache configured error_log.
Comment 2 Daniel A. R. Werner 2012-10-10 13:10:09 UTC
Sure, the problem had to be caught in resource loader already, but it could probably be propagated to QUnit to use its view for displaying it as 'missing test' there.

Anyhow, it would be nice to get a message directly on the screen or at least let all following tests still execute. It can easily happen that tests get renamed and/or someone sets something wrong in the tests registration without noticing. It just shows you what's wrong immediately without having to dig in several places first or not noticing at all.
Comment 3 Krinkle 2012-10-10 16:50:38 UTC
If you want other components to execute regardless of some components, register them as separate modules (e.g. instead of ext.foo.test, split it up like the related main modules, ext.foo.bar.test, ext.foo.baz.test, ext.foo.quux.test)

If a module is missing one or more files, executing it anyway is unacceptable and irresponsible. That will cause failures in production (undefined variables, broken layouts and what not). It is by design that modules in anyway suspicious are not loaded and (visually at least) silently ignored. This is mostly for upgrading reasons (old html cache calling inexistant modules, or new html cache but a minute-old startup module seconds after upgrading the site etc.), but there are other reasons as well.

In development, you can enable debug mode[1], in which case file handling happens on the client side and one or more missing files will just be 404 errors and the rest will execute regardless.

[1] https://www.mediawiki.org/wiki/ResourceLoader/Features#Debug_mode
Comment 4 Daniel A. R. Werner 2012-10-11 10:10:15 UTC
Thanks Timo, I think this behavior sounds reasonable. We probably should just start to separate our tests into several modules instead of having all of one extension in one module.

Thinking about this further, by the nature of unit tests, it could make sense to change the handling there and tell resource loader that all those files are independent from each other or to register them in an alternative way from how they are registered now and let resource loader put them into different modules automatically. This could be a custom module type which will report to QUnit if a file can't be loaded. Does that sound reasonable at all?

Anyhow, the later sounds like quite some work, not sure the nicer handling would be worth the effort.
Comment 5 Krinkle 2012-10-13 06:10:15 UTC
Marking this as wontfix per the following reasons:

* On a regular request, ResourceLoader does not traverse all 100s of modules and their even bigger number of components. That would be very slow and infeasible. Instead this is handled through ResourceLoader load.php on-demand and cached in the startup module.

* load.php already emits an error to the php error log and in a /* comment */ on top of the response. This response does not have an HTTP error status code, because the (rest) of the response is OK and should be handled correctly. It does include in the response (instead of the broken/incomplete module) mw.loader.state({"module.name":"error"}), which then triggers the error callbacks (if any).

* QUnit itself should imho not fail because there is no wrong assertions. Anything given to QUnit will be executed accordingly, and it can't act on what it isn't given. Responding with a QUnit test from load.php doesn't seem very appropiate either because in 99.9% of cases there will be no QUnit on the page where load.php is requested from.

* And last, if there are any missing files, this is a server-side configuration problem that the PHPUnit test will (should) cover instead. That way this only affects local development, and commits will not be merged if there are missing files.

And for local development, watching the php error log is a good habit anyway, as there are many components in MediaWiki that fail silently towards the front-end (e.g. keep using old cache if no new value can be generated from disk, or from the database etc.).

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links