T113839 is a failure of mwext-testextension-zend job on the Disambiguator extension about some issue with Metrolook. skins/Metrolook is not a dependency, but the repository was still around from a previous run.
For extensions, there is an extension_load.txt file, so we can keep the extensions between runs, but for skins there is no such things and I guess the skins are autoloaded.
So we need the job to clean up the skin dir.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | hashar | T113839 Stylesheets should not both specify "media" and contain @media | |||
| Resolved | hashar | T113841 mwext-testextension-* jobs does not remove skins from previous runs | |||
| Declined | None | T113858 MediaWiki CLI installer need an option to not inject detected skins in LocalSettings.php |
Well I created this patch https://gerrit.wikimedia.org/r/#/c/228470/ that allows skins to also be tested.
And the installer magically install them in the LocalSettings.php
## Default skin: you can change the default skin. Use the internal symbolic ## names, ie 'vector', 'monobook': $wgDefaultSkin = "metrolook"; # Enabled skins. # The following skins were automatically enabled: wfLoadSkin( 'apex' ); wfLoadSkin( 'Metrolook' );
Hum I wonder why. But if they are showing errors woulden that mean the skin is having the error its own repo too.
The CLI installer definitely injects whatever it can find under /skins/.
I am not sure why we trigger the testextension job on skins. With T68926 / https://gerrit.wikimedia.org/r/#/c/233694/ I have introduced a 'skins' entry point in PHPUnit.
A patch proposed against mediawiki/skins/Metrolook does trigger the job mwext-testextension-zend which runs:
phpunit.php --testsuite extensions
So you get the appropriate classes loaded because in MediaWiki core both extensions and skins suites are roughly equivalent:
<testsuite name="skins"> <directory>skins</directory> <directory>structure</directory> <file>suites/LessTestSuite.php</file> </testsuite>
<testsuite name="extensions"> <directory>structure</directory> <file>suites/ExtensionsTestSuite.php</file> <file>suites/ExtensionsParserTestSuite.php</file> <file>suites/LessTestSuite.php</file> </testsuite>
So it seems to me we can generalize the testextension job and have it switch the testsuite being used based on the repository triggering the change.
We also need to clean the skins between runs.
Yes. we should follow up https://gerrit.wikimedia.org/r/#/c/228470/ which adds support for testing skins.
Maybe create a task that changes the mwext-testextension- test so that it tests both extensions or skins.
Would the error that shows in the skin with that test would that mean there is a problem or is that for extensions only and skins do something differently.
Change 241314 had a related patch set uploaded (by Paladox):
Remove extension-unittests-generic test from skins
Change 241314 merged by jenkins-bot:
Remove extension-unittests-generic test from skins
I removed the skins from the slaves workspaces so it should be fine now:
salt '*-slave-*' cmd.run 'rm -fR /mnt/jenkins-workspace/workspace/*/src/skins/Metrolook' salt '*-slave-*' cmd.run 'rm -fR /mnt/jenkins-workspace/workspace/*/src/skins/apex'
Next step, delete them before running the job and or conditionally load the skins based on the list of repos that participate in the job.
We should figure out how to make the installer not add every skin to LocalSettings.php, and instead have our own skin autoloader like we already have for extensions.
Change 270712 had a related patch set uploaded (by Paladox):
Make sure the skins directory is empty in extensions unit tests and qunit tests
Change 270712 abandoned by Paladox:
Make sure the skins directory is empty in extensions unit tests and qunit tests
I belive we can close this as resolved as we support skins as deps in extensions now.