Commit graph

34 commits

Author SHA1 Message Date
jenkins-bot
cb3ae02172 Merge "resourceloader: Add @covers for stripBom and makeModuleResponse tests" 2017-06-27 13:18:10 +00:00
Timo Tijhof
580dbeba7a resourceloader: Add @covers for stripBom and makeModuleResponse tests
Change-Id: Ie32178cdd03a79a0ab871122acf081c65e5a9f4d
2017-06-26 22:15:06 -07:00
Umherirrender
be42e09aa8 build: Prepare for mediawiki/mediawiki-codesniffer to 0.9.0
The used phpcs has a bug, so the version 0.9.0 could not be enforced at the moment.
Will be fixed in next version, see T167168

Changed:
- Remove duplicate newline at end of file
- Add space between function and ( for closures
- and -> &&, or -> ||

Change-Id: I4172fb08861729bccd55aecbd07e029e2638d311
2017-06-26 17:14:31 +00:00
Timo Tijhof
84074e856d resourceloader: Add unit tests for ResourceLoader::isFileModule
Change-Id: Ic21ae9e9cc418868ad7b93fe65bd09495f38d1b2
2017-06-21 19:48:34 +00:00
daniel
2b3762ecb4 resourceloader: Allow modules to be registered via a factory callback
This should work the same way as registering API modules via a factory callback.
Point in case: Ifb8611473a971 could avoid global state using this mechanism.

Change-Id: Ifbf29006141ce2a2dff42efa352f406502a06bc6
2017-05-21 17:12:37 +00:00
Gergő Tisza
525bfbc8df Switch to librarized version of TestingAccessWrapper
Replaces \TestingAccessWrapper (defined in core) with
\Wikimedia\TestingAccessWrapper (defined in the composer package
wikimedia/testing-access-wrapper).

See https://gerrit.wikimedia.org/r/#/q/topic:librarize-testing-access-wrapper
for downstream patches.

The core version of the class is kept around for a while to avoid
circular dependency problems.

Bug: T163434
Change-Id: I52cc257e593da3d6c3b01a909e554a950225aec8
2017-04-20 14:15:57 +00:00
Timo Tijhof
16adb3fe31 resourceloader: Improve code coverage
* Missing cases for StartupModule::getModuleRegistrations
  (now 100% covered)
  - Raw modules are omitted from the manifest.
    E.g. The base modules ('jquery', 'mediawiki') are raw modules
    that we don't register client side (they can't load themselves).
  - Exceptions from getVersionHash() are caught.
  - Oversized versions are re-hashed.

* Missing cases for ResourceLoader::makeLoaderRegisterScript.
  (now 100% covered)

* Missing cases for ResourceLoader::getModule.
  (now 100% covered)

Change-Id: If9717a48195fc6ae776da5d0e86f323d7f60426d
2017-04-05 18:19:48 -07:00
Timo Tijhof
22b112aeda resourceloader: Fix testMakeModuleResponseError() failure on Travis
> 1) ResourceLoaderTest::testMakeModuleResponseError
> Failed asserting that '[e08c982d974548127cb5d7ce] Fatal exception of type Exception'
> matches PCRE pattern "/Ferry not found/".
> .../ResourceLoaderTest.php:519

This happened on Travis CI, because ResourceLoader::formatException() behaves
differently based on $wgShowExceptionDetails. Which is enabled in Vagrant
and Jenkins, but disabled by default (and thus in Travis CI builds).

Bug: T75176
Change-Id: If15dd03213703b7b6ff899cad5e5569e2515b378
2017-02-22 02:56:30 +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
6f5cb7d42b resourceloader: Add tests to verify empty string works
It's not explicitly supported anywhere, but I don't see a point in explicitly
disallowing it. Add unit tests to verify that this works.

Bug: T28804
Change-Id: I876ac43885bb27da54ef6e59b6416868ff636b84
2016-11-16 17:15:04 -08:00
Timo Tijhof
09e1b91756 resourceloader: Move 'site' and 'user' logic to makeModuleResponse
* Keep this out of makeLoaderImplementScript() to keep it more generic
  and to simplify future refactoring.

* Remove now-broken test case that asserted that the output varies
  by global debug mode.

* Make the test responsible for wrapping in XmlJsCode. Previously
  this magically happened because the module name was "user" in the
  last test case.

* Make makeLoaderImplementScript protected. It's not used anywhere
  outside ResourceLoader and we should keep it that way.

Test plan:
* Verify output unchanged:
  - load.php?modules=user&only=scripts&user=Admin (raw code)
  - load.php?modules=user&user=Admin (implement with unwrapped string)
  - load.php?modules=jquery.client (implement with closure)

Change-Id: I527d01926fb6e4ce68c931695d830cdb9ceb608c
2016-09-15 18:32:07 -07: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
Timo Tijhof
7dbad8562c resourceloader: Improve coverage in ResourceLoaderTest.php
* Fix signature of makeLoaderSourcesScript() to match
  the change in behaviour since e103ba265.

* Consistently order providers before the test.

* Simplify testRegisterValid() and remove needless @depends.

* Remove unused private method stripNoflip().

Coverage:

* Expand test coverage for register().

* Add tests for getModuleNames().

* Add tests for getModule().

* Expand test coverage for addSource().
  (case of invalid array)

* Expand test coverage for makeLoaderImplementScript().
  (case of unwrapped user script, and case of invalid scripts)

* Add tests for makeLoaderSourcesScript().

Change-Id: Ibca3e486fcd3664f171f135327a0f340ee6da9ee
2016-08-24 18:56:20 -07:00
jdlrobson
bc4e07b6f6 resourceloader: Implement modern module loading (2/2)
* Send 'module' and 'require' parameters to module closures.
  This depends on Ia925844cc22f143 being deployed one cycle earlier.
* Patch Moment and OOjs to ensure these libraries continue to expose
  their module as globals as well. AMD/UMD-compatible libraries
  only expose a global *OR* an export, not both. We need both
  for back-compat.
* Update pluralRuleParser to make use of module export to allow
  usage via require().

To test, check out the patch and run:
> mw.loader.load('moment');
> mw.loader.require('moment')()
> mw.loader.require('moment')('2011-04-01').fromNow()

Bug: T108655
Change-Id: Idbd054880ee70d659ec760aef8fcb38d0704a394
2016-04-13 15:43:41 +00:00
Kunal Mehta
6e9b4f0e9c Convert all array() syntax to []
Per wikitech-l consensus:
 https://lists.wikimedia.org/pipermail/wikitech-l/2016-February/084821.html

Notes:
* Disabled CallTimePassByReference due to false positives (T127163)

Change-Id: I2c8ce713ce6600a0bb7bf67537c87044c7a45c4b
2016-02-17 01:33:00 -08:00
Reedy
0f19a8e771 Remove duplicate array keys from tests
Change-Id: I437b87151be6589a8d5c984b90cd249c2d0ecd3c
2016-02-07 21:22:53 +00:00
Ori Livneh
e176c76da2 resourceloader: Fully remove ResourceLoaderLESSFunctions
Deprecated in 1.24, for reasons explained in a0c41ae39d. I don't see any
usage in core or extensions.

Change-Id: I46f9e04ae633e7ff1ee112b652e1865731172f1f
2015-09-04 01:41:40 +00:00
Vivek Ghaisas
9f5b6f5aeb Fix whitespace issues around parentheses
Fix issues found by MediaWiki.WhiteSpace.SpaceyParenthesis sniff.

Bug: T102617
Change-Id: Iec7f71e64081659fba373ec20d9d2006306a98f4
2015-06-16 22:14:02 +03:00
Timo Tijhof
3cf2f18bb8 resourceloader: Omit empty parameters from mw.loader.implement calls
Follows-up ebeb29723, 1f393b6da, 0e719ce23.

Also:
* Add tests for ResourceLoader::makeLoaderImplementScript().
* Apply ResourceLoader::trimArray to makeLoaderImplementScript (new in c0c221bf).

This commit changes the load.php response to omit empty parameters.

These parameters were required until recently. The client has been
updated (1f393b6da and 0e719ce23) to make these optional, thus supporting
both the old server format and the change this commit makes

Clients with a tab open from before 0e719ce23 are naturally not
compatible with load.php responses from after this commit. Ensure
this is deployed several days after 0e719ce23 to reduce race
conditions of this nature.

(This is a re-submitted version of 4ce0c0da4)

Bug: T88879
Change-Id: I9e998261ee9b0b745e3339bc3493755c0cb04b6a
2015-04-03 07:20:51 +01:00
Timo Tijhof
dbd718dc55 resourceloader: Add @covers and minor clean up of test suites
* Move testMixedCssAnnotations to ResourceLoaderFileModuleTest.
* Re-order data providers before test methods.
* Add relevant @covers annotations in resourceloader/ tests.
* Make test helper function private.
* Add a few @covers for methods called from OutputPage::makeResourceLoaderLink
  (only one level deep, we should have separate unit tests for
  the more internal helpers).

Change-Id: I2cc1757126214ed28059d4566ca813a86bcd95a7
2015-03-23 04:16:08 +00:00
Kunal Mehta
cb65b994db resourceloader: Add ResourceLoader::isModuleRegistered()
Currently if code wants to check whether a module is registered it
has to call getModule() and see if the response !== null.

Change-Id: I4b470083ddaa5d8cd6be50d5c5b690d4b99b6c4a
2015-03-15 02:45:43 +00:00
Timo Tijhof
54473cbb6d resourceloader: Call setName() in test suite before calling getStyles()
This caused a database error due to NULL being inserted as name, which is illegal.

> Function: DatabaseSqlite::replace/single-row
> NOT NULL constraint failed: unittest_module_deps.md_module
> Stack trace:
> #3 includes/resourceloader/ResourceLoaderFileModule.php(420): DatabaseSqlite->replace()
> #4 tests/phpunit/includes/resourceloader/ResourceLoaderTest.php(88): ResourceLoaderFileModule->getStyles()
> #5 (): ResourceLoaderTest->testLessFileCompilation()

This test shouldn't be trigggering database updates, but that's for
a later change to stub out or refactor.

Bug: T91567
Change-Id: Ic451bd41e2ffc188d2efd6b7ce61b03b9de61296
2015-03-06 03:26:44 +01:00
Krinkle
6e46b4b380 Revert "resourceloader: Omit empty parameters from mw.loader.implement calls"
Works as intended, but didn't account for the first implement() parameter
being omitted client-side. Revert until that is accounted for, then re-try after
that fix is rolled out for > 1 week.

This reverts commit 4ce0c0da42.

Change-Id: I36c1619991663c0303636d1d3f037b0021ac79bf
2015-01-20 23:56:17 +00:00
Timo Tijhof
4ce0c0da42 resourceloader: Omit empty parameters from mw.loader.implement calls
Follows-up ebeb297236.

Also:
* Add tests for ResourceLoader::makeLoaderImplementScript().
* Apply ResourceLoader::trimArray to makeLoaderImplementScript (new in c0c221bf).

As always, the client (updated in Ie32e7d6a3c) is backward-compatible with old
(cached) load.php module responses. However, the old client is not compatible
new load.php responses after this commit.

That's generally not an issue, as we don't cache the client very long (~ 5 min).
However people with their browser open and mw.loader clients initialised can
still make new module requests (e.g. modules loaded on-demand, such as when
previewing edits, clicking buttons etc.). That can easily be several hours after
initial page load. As such, client/server bound changes should always be
back-compat and deployed a reasonable time apart to reduce chances of active
sessions making subsequent requests. Ideally we'd find some solution to this in
the long-term, but handling this at all is better than what we usually do...

For deployment: Ensure this is deployed several days after Ie32e7d6a3c09f.

Change-Id: Ic8d7efe49b5d45e3f95a2f04e3a26a014b10af16
2015-01-20 23:11:05 +00:00
Kunal Mehta
0e05ec5b31 Don't create Language objects during ResourceLoader tests
Mock calls to ResourceLoaderContext::getDirection(), which creates
Language objects to get the directionality of a language.

Change-Id: Ibe6da3013e658aa7cf596c1da2f8ca1314b7cdd3
2014-12-18 16:52:28 -08:00
umherirrender
ef46373d70 Refactor hook handling in ResourceLoaderTest
No need to set a global $wgHooks, just set it for the current test.

Change-Id: Ic3e82608efa617a5a7f7c31365d748222bc6d6da
2014-10-23 19:19:18 +00:00
Bartosz Dziewoński
4c01f8b2bc Make "/*@noflip*/ /*@embed*/" annotation work without CSSJanus hacks
This reverts most of commit 2d842f1425,
leaving only the test added in it, and reimplements the same
functionality better.

Instead of stripping /*@noflip*/ annotations in CSSJanus, which is
incompatible with other implementations that preserve it, extend
CSSMin to allow other CSS comments to be present before the
rule-global @embed annotation. (This required making the regex logic
in it even worse than it was, but it's actually slightly less terrible
than I expected it would be. Good thing we have tests!)

Bug: 69698
Change-Id: I58603ef64f7d7cdc6461b34721a4d6b15f15ad79
2014-09-23 22:47:54 +00:00
jenkins-bot
d1b6cd35d4 Merge "resourceloader: Only store sources' load.php urls" 2014-09-07 23:10:33 +00:00
Kunal Mehta
e103ba265b resourceloader: Only store sources' load.php urls
Previously ResourceLoader would store any arbitrary data about a
source, provided it had a 'loadScript' key. It would register
the 'local' source with an additional 'apiScript' key, which was
also documented in DefaultSettings.php. However, it was
completely unused outside of the ForeignAPIGadgetRepo class in
Gadgets 2.0, which should be changed to take an API url as a
parameter. This was not useful as it was not ever formally
exposed, and it could not be depended upon that a source had
registered an 'apiScript' key.

For backwards compatability, both ResourceLoader::addSource()
and mw.loader.addSource() will both take an array/object, but
discard all parameters except for 'loadScript'.

Also added tests for ResourceLoader::addSource().

Bug: 69878
Change-Id: I4205cf788cddeec13b619be0c3576197dec1b8bf
2014-09-05 04:38:22 +02:00
Bartosz Dziewoński
2d842f1425 Make "/*@noflip*/ /*@embed*/" annotation work
To do it, just remove /*@noflip*/ annotations in CSSJanus after
we're done processing. They are not needed anymore and some obscure
interactions with CSSMin logic for preserving comments caused
`/*@noflip*/ /*@embed*/ background-image: url(…)` not to work
correctly (it would not be embedded).

This also requires us to always do CSSJanus processing, even when we
don't need flipping, to consistently handle the annotations.
I'm not entirely sure if this is worth it, but I still greatly prefer
doing it to documenting this stupid limitation. :)

Bug: 69698
Change-Id: I311b12b08b2dff9d45efb584db08cf4a11318f59
2014-08-18 17:40:51 +02:00
Bartosz Dziewoński
90c13419f0 Add a test for mixed /*@noflip*/ and /*@embed*/ CSS annotations
Given that we have entirely separate code for handling one and the
other, and given the nature of code comments stuffed inside other
structures, this isn't really obvious that they work.

And indeed, "/*@noflip*/ /*@embed*/" doesn't work (filed bug 69698).
Amazingly, all other combinations do.

Change-Id: Ie30bab251eb4abee122c783d057de4102e53d1fc
2014-08-18 16:40:25 +02:00
Kunal Mehta
126fb8d157 OutputPage: Support foreign module sources in makeResourceLoaderLink
To do so, created ResourceLoader::createLoaderURL(), which takes a
ResourceLoaderContext object. ResourceLoader::makeLoaderURL() was
deprecated.

While reviewing usage of the old function, many of the callers only
differed by one or two parameters from their respective
ResourceLoaderContext object. To simplify that use case, I created
DerivativeResourceLoaderContext, based of off DerivativeContext for
IContextSource.

Change-Id: I961c641ab953153057be3e3b8cf6c07435e9a0b0
2014-07-19 23:44:00 +00:00
Siebrand Mazeland
69ec133bc5 Pass phpcs-strict on some test files (10/11)
Change-Id: I5624292143fcabe890779f5095eae735d7afb176
2014-04-24 13:50:56 -07:00
Timo Tijhof
9976cef4ed tests: Add ResourceLoaderTestCase and abstract context creation
Change-Id: Ib4b265256e60a2f2109da73dc7edba6a75587ce2
2014-03-07 20:09:59 +01:00
Renamed from tests/phpunit/includes/ResourceLoaderTest.php (Browse further)