Commit graph

4 commits

Author SHA1 Message Date
Kunal Mehta
75160bdd3b Use MediaWikiCoversValidator for tests that don't use MediaWikiTestCase
Change-Id: I8c4de7e9c72c9969088666007b54c6fd23f6cc13
2018-01-01 08:28:02 +00:00
Timo Tijhof
466939c631 resourceloader: Don't let module exception break startup
When getScript (or some other method used in a module response)
throws an error, only that module fails (by outputting mw.loader.state
instead of mw.loader.implement). Other modules will work.

This has always been the case and is working fine. For example,
"load.php?modules=foo|bar", where 'foo' throws, will return:

```js
/* exception message: .. */
mw.loader.implement('bar', ..)
mw.loader.state('foo', 'error')
```

The problem, however, is that during the generation of the startup
module, we iterate over all other modules. In 2011, the
getVersionHash method (then: getModifiedTime) was fairly simple
and unlikely to throw errors.

Nowadays, some modules use enableModuleContentVersion which will
involve the same code path as for regular module responses.

The try/catch in ResourceLoader::makeModuleResponse() suffices
for the case of loading modules other than startup. But when
loading the startup module, and an exception happens in getVersionHash,
then the entire startup response is replaced with an exception comment.

Example case:
* A file not existing for a FileModule subclass that uses
  enableModuleContentVersion.
* A database error from a data module, like CiteDataModule or
  CNChoiceData.

Changes:
* Ensure E-Tag is still useful while an error happens in production
  because we respond with 200 OK and one error isn't the same as
  another.
  Fixed by try/catch in getCombinedVersion.
* Ensure start manifest isn't disrupted by one broken module.
  Fixed by try/catch in StartupModule::getModuleRegistrations().

Tests:
* testMakeModuleResponseError: The case that already worked fined.
* testMakeModuleResponseStartupError: The case fixed in this commit.
* testGetCombinedVersion: The case fixed in this commit for E-Tag.

Bug: T152266
Change-Id: Ice4ede5ea594bf3fa591134bc9382bd9c24e2f39
2016-12-15 23:25:57 +00:00
Timo Tijhof
4dd8c9606e resourceloader: Add test coverage for ResourceLoaderContext::msg()
Brings ResourceLoaderContext coverage to 100%.

Change-Id: I3ccc6aa87c89f69158b8d8684be565403afb4036
2016-12-01 20:47:05 -08:00
Timo Tijhof
c4e5cc2957 resourceloader: Create unit tests for ResourceLoaderContext
* Fix up one last use of global config vars in this class.
  Other places in this class already used $rl->getConfig().
  This way we don't inherit all of MediaWikiTestCase.

* Add unit tests covering all of ResourceLoaderContext
  except expandModuleNames and getImageObj (tested in better
  places already with the right @covers).

* Increase coverage for expandModuleNames(), add missing case
  of when modules are not in alphabetical order.

Change-Id: Id19b084d37a6c3a77b36e03509adffb6b156fee1
2016-08-29 16:47:15 -07:00