Commit graph

40 commits

Author SHA1 Message Date
Tim Starling
e08ea8ccb9 ResourceLoader namespace
Move ResourceLoader classes to their own namespace. Strip the
"ResourceLoader" prefix from all except ResourceLoader and
ResourceLoaderContext.

Move the tests by analogy.

I used a namespace alias "RL" in some callers since RL\Module is less
ambiguous at the call site than just "Module".

I did not address DependencyStore which continues to have a non-standard
location and namespace.

Change-Id: I92998ae6a82e0b935c13e02a183e7c324fa410a3
2022-05-16 14:41:27 +10:00
jenkins-bot
3801581dae Merge "resourceloader: Preserve new 'debug' param in getScriptURLsForDebug()" 2021-09-28 15:27:27 +00:00
Timo Tijhof
6d14529c69 resourceloader: Preserve new 'debug' param in getScriptURLsForDebug()
Follows-up Ieaf04e0c289646dd5 which changed internal references to
bool(true) for 'debug' to the integer DEBUG_ constants, and introduced
a new debug=2 parameter.

In the refactor, I missed the setDebug() calls for
DerivativeResourceLoaderContext, which were still passing a boolean,
but more importantly were effectively passing debug=1 even if the
pageview carried debug=2. This isn't a problem yet in production since
debug=2 is currently identical in behaviour to debug=1.

The impact of this issue is mainly noticed through secondary CSS
requests. The URLs for primary stylesheets, and JS modules was already
forwarding the current "debug" version.

Test Plan:
* Open Main_Page?action=edit&debug=2
* Before this patch, e.g. on mediawiki.org today, secondary
  stylesheet requests (part of a JS module) have debug=1.
  For example "modules=jquery.makeCollapsible.styles&only=styles".
* After this, everything has debug=2 when the page view has debug=2.

Bug: T85805
Change-Id: Ia8fba4e30397bc5890033f13417b6739b0f83c38
2021-09-25 20:06:42 +00:00
Timo Tijhof
fc5403e6a8 resourceloader: Add test for getVersionHash() in debug mode
Folllows-up I0e63eef4f85b13.

Bug: T235672
Change-Id: I012ea8b1a40646ce7522c26d11139eddaa2067d1
2021-09-25 00:08:51 +01:00
Timo Tijhof
008b6528b6 resourceloader: Skip version hash calculation in debug mode
=== Why

* More speed

  In debug mode, the server should regenerate the startup manifest
  on each page view to ensure immediate effect of changes. But,
  this also means more version recomputation work on the server.

  For most modules, this was already quite fast on repeat views
  because of OS-level file caches, and our file-hash caches and
  LESS compile caches in php-apcu from ResourceLoader.
  But, this makes it even faster.

* Better integration with browser devtools.

  Breakpoints stay more consistently across browsers when the
  URL stays the same even after you have changed the file and
  reloaded the page. For static files, I believe most browsers ignore
  query parameters. But for package files that come from load.php,
  this was harder for browsers to guess correctly which old script URL
  is logically replaced by a different one on the next page view.

=== How

Change Module::getVersionHash to return empty strings in debug mode.

I considered approaching this from StartupModule::getModuleRegistrations
instead to make the change apply only to the client-side manifest.

I decided against this because we have other calls to getVersionHash
on the server-side (such as for E-Tag calculation, and formatting
cross-wiki URLs) which would then not match the version queries that
mw.loader formats in debug mode.

Also, those calls would still be incurring some the avoidable costs.

=== Notes

* The two test cases for verifying the graceful fallback in production
  if version hash computations throw an exception, were moved to a
  non-debug test case as no longer happen now during the debug
  (unminified) test cases.

* Avoid "PHP Notice: Undefined offset 0" in testMakeModuleResponseStartupError
  by adding a fallback to empty string so that if the test fails,
  it fails in a more useful way instead of aborting with this error
  before the assertion happens. (Since PHPUnit generally stops on the
  first error.)

* In practice, there are still "version" query parameters and E-Tag
  headers in debug mode. These are not module versions, but URL
  "combined versions" crafted by getCombinedVersion() in JS and PHP.
  These return the constant "ztntf" in debug mode, which is the hash
  of an empty string. We could alter these methods to special-case
  when all inputs are and join to a still-empty string, or maybe we
  just leave them be. I've done the latter for now.

Bug: T235672
Bug: T85805
Change-Id: I0e63eef4f85b13089a0aa3806a5b6f821d527a92
2021-09-15 18:13:09 -07:00
Timo Tijhof
7c6713b4d9 resourceloader: Make getVersionHash() final
This is in preparation for making all version hashes the empty string
in debug mode, which will make things faster to work with, but also
helps solve T235672 in an easy way client-side without having to
add additional complexity to the mw.loader client that is specific
to debug mode.

Bug: T235672
Change-Id: I43204f22dfbcf5d236b35adc5b35df3da8021bad
2021-09-15 18:10:49 -07:00
Umherirrender
2e4ee47c3d Cleanup mixed space/tab line indent
Change-Id: I833052a656b1ce419c0929f6f0514f2a33c2c4cc
2021-09-04 00:52:31 +02:00
Timo Tijhof
b7c70526a9 resourceloader: Add missing Module->setConfig() calls in tests and installer
There is a fallback in Module->getConfig() to the global services
container. This is not meant to be used in practice, but there were
two places where this was missing: WebInstallerOutput, and various
PHPUnit tests.

* Add missing setConfig() to WebInstallerOutput.

* Add missing setConfig() to various tests. Most tests were already
  doing this correctly and using the standard mock from
  ResourceLoaderTestCase. Upon switching the last few tests as well,
  I uncovered various errors due to the mock missing some settings
  that the tested code uses, so these have been added now to
  ResourceLoaderTestCase.

Bug: T277728
Change-Id: I44f16ec4e00423fb6f641e58fffc1d40e4571f01
2021-06-13 21:20:58 +00:00
Daimona Eaytoy
535d7abf59 phpunit: Mass-replace setMethods with onlyMethods and adjust
Ended up using
  grep -Prl '\->setMethods\(' . | xargs sed -r -i 's/setMethods\(/onlyMethods\(/g'

special-casing setMethods( null ) -> onlyMethods( [] )

and then manual fix of failing test (from PS2 onwards).

Bug: T278010
Change-Id: I012dca7ae774bb430c1c44d50991ba0b633353f1
2021-04-16 20:15:00 +02:00
Gergő Tisza
fab70ba24b resourceloader: Mention ECMAScript version in JS parse error
ResourceLoader requires user scripts to be valid ES5.

Browsers have implemented most of the ES6 spec by now, with some
of the ES6 features actually having been around for a long time now.

As such, parse errors for otherwise valid ES6 scripts, from the POV of
the script author (which can include people editing their user scripts),
this error message can be inscrutable. Try to make it more explicit.

Change-Id: I20a6c705ac44c2c41b11e4a8f16675592b6d8a87
2020-06-01 23:37:57 +00:00
Max Semenik
dd7026585c Backwards-compatible PHPUnit 8 preparations
This commit splits changes from Ic14f5debc53e55d6714 to reduce it to
only strictly needed things. It can be merged immediately.

Bug: T192167
Change-Id: I8c541a66ea13421dbe7fa51d197d5455cc4786eb
2019-11-02 12:40:07 -07:00
Daimona Eaytoy
ef5ab69629 Replace setExpectedException with two args
Find: ^(\t*)(\$this->)setExpectedException\(\s+(\\?[a-z\\]+::class),\s+('(?:[^'\\]|\\')+'|"(?:[^"\\]|\\")+")\s+\);

Replace: $1$2expectException( $3 );\n$1\$this->expectExceptionMessage( $4 );

+broke long lines manually.

Bug: T192167
Change-Id: I5557b4372625def55a53ac637c2f980f51f12933
2019-10-05 16:14:05 +00:00
Timo Tijhof
d6dd6e4d72 resourceloader: Remove use of object registering in test suites
This was done as a "clever" shortcut to make sure tests a little
but shorter, but also made them less consistent with normal code.

Remove this in favour of 'class' or 'factory' options as needed.
Also remove a bunch of unneeded register() calls.

The tests cover everything affected by this change.

Side fix - isFileModule should reject modules with 'factory'
the same way it rejected raw objects and non-FileModule 'class'
cases already. This is now covered by tests as well.

Bug: T222637
Change-Id: I3996317dbcd780cc6e0f82c84e769c08a3fc42bb
2019-07-12 01:17:44 +00:00
Timo Tijhof
469d961bb7 resourceloader: Switch Xml::encodeJsCall call in getDeprecationInformation
Also rename one of the test classes to have a more descriptive name.

Bug: T32956
Change-Id: I1a970c198300b7ef0d99c15609f1fb7fa8783b98
2019-06-19 15:54:08 +01:00
Timo Tijhof
e42837e977 resourceloader: Remove selective build optimisation from getModuleContent()
This follows 5ddd7f91c7, which factored out response building
from ResourceLoader.php to ResourceLoaderModule::buildContent.
As optimisation, I made this method only return the array keys
needed for the current response; based $context->getOnly().

The reason for this refactoring was the creation of the
'enableModuleContentVersion' option to getVersionHash(), which
would use this method to create a module response, and hash it.

During the implementation of that option, I ran into a problem.
getVersionHash() is called by the startup module for each
registered module, to create the manifest. The context for the
StartupModule request itself has "only=scripts". But, we must
still compute the version hashes for whole modules, not just
their scripts.

I worked around that problem in aac831f9fa by creating a mock
context in getVersionHash() that stubs out the 'only' parameter.

This worked, but made the assumption that the scripts and styles
of a module cannot differ based on the 'only' parameter.
This assumption was wrong, because the 'only' parameter is part
of ResourceLoaderContext and available to all getters to vary on.
Fortunately, the 'enableModuleContentVersion' option is off by
default and nobody currently using it was differing its output
by the 'only' parameter.

I intend to make use of the 'enableModuleContentVersion' option
in StartupModule to fix T201686. And StartupModule outputs a
manifest if the request specifies only=scripts, and outputs
a warning otherwise. As such, it cannot compute its version
if the 'only' parameter is stubbed out.

* Remove the 'only' parameter stubbing.
* Remove the selective building from the buildContent() method.
  This was not very useful because we need to build the whole
  module regardless when computing the version.

As benefit, this means the in-process cache is now shared between
the call from getVersionHash and the call from makeModuleResponse.

Bug: T201686
Change-Id: I8a17888f95f86ac795bc2de43086225b8a8f4b78
2018-08-30 21:02:57 +00:00
Timo Tijhof
ae9c32b508 resourceloader: Add test for getVersionHash parent-definition requirement
Change-Id: I69cdfea96c1e64bd8a7495eb6e56d0aefbe37643
2018-06-26 16:24:31 +01:00
Timo Tijhof
96d25f6bcc resourceloader: Remove support for Module::getModifiedTime() and getModifiedHash()
Deprecated since 1.26. No subclasses in Wikimedia Git define these methods,
no calls to methods by this name in Wikimedia Git.

If a module subclass were to still define such a method, it is simply
not called anymore. The version hash system introduced in 1.26 will
still invalidate modules based on wgCacheEpoch.

Bug: T94074
Change-Id: I65b2a625a30f22c8a9d14a3505605546fa5bab83
2018-06-05 02:02:06 +01:00
Timo Tijhof
42d537503d resourceloader: Add @covers for covered deprecated methods
These are deprecated for extensions to implement, but still
supported and already triggered by the test cases. Previously
not mentioned because they were ignored by PHPUnit, but not anymore.

Change-Id: I594788e322bfd83be1e7847d3272d57c549f3e8b
2018-04-17 01:40:48 +01:00
Umherirrender
45da581551 Use ::class to resolve class names in tests
This helps to find renamed or misspelled classes earlier.
Phan will check the class names

Change-Id: Ie541a7baae10ab6f5c13f95ac2ff6598b8f8950c
2018-01-26 22:49:13 +01:00
Timo Tijhof
3d342d4deb resourceloader: Add support for modules sending preload headers
ResourceLoaderModule objects gain a new method getPreloadLinks() which
returns an array with the meta data required to build a Link rel=preload
header according to the current draft for W3C Preload.
<https://w3c.github.io/preload/>

Another implementation of this is already in use in OutputPage for
preloading the logo image.

This array is formatted by the ResourceLoaderModule::getHeaders method,
which is implemented as "final" at this time, thus restricting use to
the Link rel=preload header.

Headers are exposed and process-cached, like all other content
(scripts, styles, etc.), through ResourceLoaderModule::getModuleContent,
and aggregated by ResoureLoader::makeModuleResponse.

I had hoped for the getPreloadLinks to be stateless (not vary on $context).
Whether something should be preloaded and what, should not vary on the
skin or language. However, while that conceptually holds true, the exact
url for any given resource may still vary. Even the main use case for this
feature (T164299, preloading base modules request) require $context to pass
down skin and lang to the load.php url.

Add full test coverage and example documentation.

Bug: T164299
Change-Id: I2bfe0796ceaa0c82579c501f5b10e931f2175681
2017-08-03 03:57:00 +00:00
Timo Tijhof
483f13b226 resourceloader: Use "\n" instead of ";" as separator for scripts
This fixes two bugs:

* 1) When two modules are requested, and the first one ends with ";"
     inside a comment, the second module might become part of
     that comment.
* 2) A request with script=only where the requested module content
     ends in a statement without ";", but has a comment after it
     that does ends with a semicolon, then in debug=false, mw.loader.state()
     would be appended directly after the semicolon-less statement because
     the check is performed before minification.
     Previously:
     script> foo()
     script> // bar();
     states> mw.loader.state( {} );
     Became (minified separately):
     script> foo()
     states> mw.loader.state({});
     Became (concatenated)
     > foo()mw.loader.state();
     Which is invalid code.

Both of these are now fixed.

Bug: T162719
Change-Id: Ic8114c46ce232f5869400eaa40d3027003550533
2017-06-28 03:59:05 +00:00
Timo Tijhof
d3e31c7ea4 resourceloader: Add basic tests for getScript() and buildContent()
Bug: T162719
Change-Id: I37d64da77682adfef61e78033d639b623d7c9c2b
2017-06-26 22:15:17 -07:00
Fomafix
639176e01b ResourceLoaderModuleTest: Exchange expected and actual parameter
For assertEquals of PHPUnit the first parameter is $expected and the second
parameter is $actual.
https://phpunit.de/manual/current/en/appendixes.assertions.html#appendixes.assertions.assertEquals

Change-Id: Iad4b37ee74a03aa00f2dc14d3c474796b3191b51
2017-03-31 22:29:03 +02: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
Timo Tijhof
9446f0cb2b resourceloader: Re-enable getVersionHash test
Shouldn't be flaky anymore as of 5d5b269e0e.

This reverts commit d151dc40ae.

Bug: T109394
Change-Id: Id94b6e71a747a5b9fe1fecd8145955fde4f39a9c
2015-12-09 20:52:58 +00:00
umherirrender
1647bb07fc Fix phpunit for wikis with $wgResourceLoaderValidateJS = false
1) ResourceLoaderModuleTest::testValidateScriptFile
Replace invalid syntax with error logging
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'var a = 'this is';
- {
-invalid'
+'mw.log.error("JavaScript parse error: Parse error: Unexpected token;
token } expected in file 'input' on line 3");'

Change-Id: I0271de4bf2d5bcc50eaf5c0e999e16e554985805
2015-12-07 17:38:28 +01:00
Timo Tijhof
d151dc40ae resourceloader: Disable flaky test ResourceLoaderModule::getVersionHash
Bug: T109394
Change-Id: I8e62d9e8326046a895588d9cc63f47e6c0d12eda
2015-10-20 02:58:12 +01:00
Timo Tijhof
047b60b96d resourceloader: Store relative instead of absolute paths in module_deps
Make paths stored in the module_deps table relative to $IP. This ensures that when
the MediaWiki install path changes and/or if the location of the extension or skins
directory changes, that ResourceLoader's internal model is still accurate.

Previously when these paths change, ResourceLoader would exhibit various bugs.

1. Unable to detect changes in the module (if the directory no longer exists).
2. Point #1 is usually preceeded by one last cache invalidation as the content hash
   of the file path changes (from a valid hash to an empty string).
3. Unnecessary cache invalidation (if both old and new directories exist). This
   happens when a file is both an entry point (in the 'scripts' or 'styles' array)
   and also a file dependency. At first they are de-duplicated by array_unique.
   But after the disk path changes, the next check will result in the old path
   being fetched from module_deps, and the new path from the live configuration.
   This causes two changes that result in needless cache invalidation:
   - The hash list contains one more item (T111481).
   - The hash list contains both the old and new version of a file.
     (or even alternate versions, e.g. when a change is backported to the old
     wmf branch; it also affects wikis on the new branch due to the stale
     file path still in the database).

It seems unusual to move a MediaWiki install, and usually we recommend third
parties to run update.php if site administrators do move their wiki. However
Wikimedia's deployment system implicitly moves the MediaWiki install continously
from e.g. "/srv/mediawiki/php-1.26wmf5" to "/srv/mediawiki/php-1.26wmf6".

This caused virtually all ResourceLoader caching layers to get invalidated every
week when another wmf-branch is deployed, thus breaking these file paths, which
changes the version hash, which then invalidates the cache.

Bug: T111481
Change-Id: I173a9820b3067c4a6598a4c8d77e239797d2659c
2015-09-30 00:25:27 +00:00
Reedy
58f0a7ee4e Wrap some long strings in tests/
Change-Id: I89d53c5051e5ee4bd8624df8ee2b25993090a7df
2015-09-26 21:01:59 +01:00
Timo Tijhof
370a7d5f39 resourceloader: Make tests less susceptible to timestamp races
getDefinitionSummary isn't the authoritive method to detect changes.
Using it as such may false cause something to appear detected or
undetected. Use getVersionHash() instead.

Thanks to Gilles for uncovering this bug.

Bug: T105476
Change-Id: Ibefc9fa8ffd9d45e29901d726801e8d4e008b66f
2015-07-10 13:59:52 +00:00
Timo Tijhof
1e4336d435 resourceloader: Add unit test for validateScriptFile()
Follows-up cd0dff5c00.

Change-Id: Ie208e58053048e932ef3f61f849148b1d88bc0be
2015-06-02 23:50:57 +01: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
Kunal Mehta
fe5170d32b Move tests into separate ResourceLoaderFileModuleTest
Change-Id: I2504867850b7e6bda2afb8c4fedbe6b8819bc15b
2014-10-22 17:16:47 +00:00
umherirrender
63dc5abc9a Fixed spacing
- Added space after reserved words: function, foreach, if
- Combined 'else if' into elseif
- Added braces to one-line statements
- Added spaces after comma, before parentheses

Change-Id: Ie5bbf680d6fbe0f0872dab2700c16b1394906a72
2014-08-27 18:31:50 +02:00
addshore
dcaa81c743 Fix phpcs errors in tests dir
Change-Id: I79fa3b8f92e958f4a0dc4fe892703f37d711ca95
2014-08-17 22:57:09 +01:00
Kunal Mehta
6a826fd6bf Register a fake skin instead of a 'vector' that will fail
Bug: 69639
Change-Id: Icd87520f7b6de337b009144420c3a430861d0833
2014-08-15 22:50:25 -07:00
Kunal Mehta
1154e1848f SkinFactory: register skins in Setup.php
This un-makes $wgValidSkinNames a legacy thing, and is
more backwards-compatible friendly.

Change-Id: I5c442f3c9e4ee7a4a3980fd02138ee756ef9fa7a
2014-08-13 13:43:09 +02:00
Bartosz Dziewoński
bfe9614992 ResourceLoaderModuleTest: Ensure 'vector' is a valid skin
Change-Id: I4c121339aec80a4c56042de7be85cf5f1748b9f0
2014-07-23 21:18:19 +00:00
Sam Smith
0bb3d2a499 Make ResourceLoaderFileModule#getAllStyleFiles include all skin styles
* Add the ResourceLoaderFileModule#getAllSkinStyleFiles method,
  which returns all of the skinStyles files for a given module
* The LessFileCompilationTest and checkLess.php script, which use
  the #getAllStyleFile method, now validate all LESS style files

Bug: 63343
Change-Id: Ib2eb5761919c648aea4ae58f8d0531799fe7982b
2014-06-17 19:38:58 +00: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/ResourceLoaderModuleTest.php (Browse further)