Page MenuHomePhabricator

Move bundlesize test from npm script to MediaWikiIntegrationTest
Closed, ResolvedPublic

Description

As Readers Web works on the desktop improvements project, we’d like to keep track of the byte sizes of individual ResourceLoader modules inside Vector in order to avoid any performance regressions.

We've written a Node script that fetches each ResourceLoader module and runs it through the bundlesize library to validate it’s payload size.

https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/Vector/+/602780/

This script can be converted to a MediaWikiIntegrationTest.

Event Timeline

On the topic of tracking the size of assets: last week WMDE added some historical tracking of two of their components. The result can be seen at:

https://doc.wikimedia.org/Wikibase/master/js/data-bridge-dist-size/

https://doc.wikimedia.org/Wikibase/master/js/tainted-ref-dist-size/

That uses Github API to retrieve the data and looks very neat. Their task was https://phabricator.wikimedia.org/T253204 and the change https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Wikibase/+/594122/

But that might be an entirely different thing ;)

From a quick read at the task, using PHP / MediaWiki one should be able to extract each of the registered RL modules and then run bundlesize against the flat files.

As for which command to run, Quibble has the ability setup the whole environment and runs arbitrary commands. A minimal use case:

quibble --command /bin/bash

Would clone repos, install dependencies, boot mysql/php -s/chromedriver and drop you in a shell. We use that in specific Jenkins jobs for use case we don't have much interest in implementing inside Quibble itself. An example is https://integration.wikimedia.org/ci/job/mediawiki-fresnel-patch-docker which runs some performance tests. The job invokes:

quibble --packages-source vendor --db mysql --db-dir /workspace/db --skip-deps --commands "mediawiki-fresnel-patch"

The mediawiki-fresnel-patch script being provided by the CI team and is maintained along the Docker image.

So plenty of possibilities, we would need to investigate a bit more to have a better understanding of what has to run. The implementation itself should be rather easy :)

Summary from a discussion with Timo. WMDE writes node module and track it after merge for long time perspective. Webteam writes a MediaWiki extension and wants to enforce it on patchsets.

The easiest thing would be to write this as a PHP structure test, but if that can't work for some reason, we'd need to build you a custom job that runs npm test inside quibble (which we don't normally run).

The npm-test job is expected to be standalone, both to maitain consistency for CI management (building and maintaining a custom Jenkins CI image for every product does not scale), as well as for developer ergonomics in the form of reproducibility for local development. E.g. that when you run npm test in an isolated local environment, it Just Works. It does not depend on something else being pre-installed elsewhere locally and connected.

This task is essentially asking for a Quibble job that run shell command to execute a repo-local script (Quibble being our abstraction for setting up a database and web server with MediaWiki). We already have two jobs that do this:

  • A Node.js one, which runs selenium-test. This seems to be working today. Is there a problem with this currently?
  • A PHP one, which runs PHPUnit.

The way I would recommend implementing this, if asked today without prior art, is as a simple PHPUnit structure test in Vector, which uses ResourceLoader->getModule() to build and measure build sizes. It can then compare them to constant or JSON entry, and fail accordingly as needed. I could work on this with @vas, for example.

As part of T244276 they have added npm -s run test:size to selenium-test in both MinervaNeue and Vector. The selenium-test is meant to run integration tests across multiple repositories and as a result the test:size is run for any change made to a repository that depends on one of those two skins (ie pretty much everything). A change to mediawiki/core thus end up running npm install and npm run test:size for Vector. It is a bit of a waste.

I thought I had vetoed the whole idea of asserting the size of resources by having to spawn a whole MediaWiki setup, and instead suggested to use PHPUnit to generate the assets from MediaWiki ResourceLoader and then asserts from there. Anyway we need:

  • a dedicated job for both those repositories
  • remove test:size from the selenium-test entry point

Change 639166 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] Job to run npm run test:size inside Quibble env

https://gerrit.wikimedia.org/r/639166

hashar added a subscriber: Jdlrobson.

Looks like I forgot about this task.

The test:size script has been added to the CI entry point selenium-test with:

Quibble crawls all extensions/skins repositories and will run selenium-test for each of those having one. Since all repositories depend on Vector, the test:size script is run for any change beside the ones targeting Vector.

The idea is to move test:size out of selenium-test and instead execute it in a standalone job that set up MediaWiki and does cd skins/Vector && npm run-script test:size.

Since all repositories depend on Vector, the test:size script is run for any change beside the ones targeting Vector.

Note changes in core can impact the bundle sizes in Vector and it's good if those increase bundle sizes in Vector unexpectedly and the job fails. I agree it's probably useless if run in another repo such as Wikibase though and I guess if run in a dedicated script that could be configured to also run for core changes?

That is a good point, I will look into at adding jobs for mediawiki/core.

Change 736465 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/skins/Vector@master] Move bundlesize test to MediaWikiIntegration test

https://gerrit.wikimedia.org/r/736465

I didn't know about this task, but came to the same conclusion as others here about doing these bundlesize checks in PHPUnit. The patch above does this for Vector.

Change 639166 abandoned by Hashar:

[integration/config@master] Job to run npm run test:size inside Quibble env

Reason:

It is being moved to a PHPUnit test: https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/736465

https://gerrit.wikimedia.org/r/639166

Ah that is great. Using PHPUnit was my suggestion on the original change that went using JavaScript / API requests, so I am quite happy to see it moving forward! There is the same use case for MinervaNeue.

Ah that is great. Using PHPUnit was my suggestion on the original change that went using JavaScript / API requests, so I am quite happy to see it moving forward! There is the same use case for MinervaNeue.

Yes, if @Jdlrobson approves it, I (or whoever) can make the same change for Minerva.

Change 737767 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@master] tests: Add structure test for bundlesize.config.json

https://gerrit.wikimedia.org/r/737767

Change 740834 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/skins/MinervaNeue@master] Move bundlesize test to MediaWikiIntegration test

https://gerrit.wikimedia.org/r/740834

kostajh renamed this task from Add a CI Job for running bundlesize tests in Vector to Move bundlesize test from npm script to MediaWikiIntegrationTest.Nov 23 2021, 1:55 PM
kostajh claimed this task.
kostajh updated the task description. (Show Details)

Change 737767 merged by jenkins-bot:

[mediawiki/core@master] tests: Add structure test for bundlesize.config.json

https://gerrit.wikimedia.org/r/737767

Change 740834 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Move bundlesize test to MediaWikiIntegration test

https://gerrit.wikimedia.org/r/740834

Change 736465 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Move bundlesize test to MediaWikiIntegration test

https://gerrit.wikimedia.org/r/736465

Php code coverage test on core failed, seems to be related to this task.
Backtrace:

22:38:27 [de95d234e4a84fda36953fc6] [no req]   Error: Class 'MediaWiki\Tests\Structure\BundleSizeTest' not found
22:38:27 Backtrace:
22:38:27 from /workspace/src/skins/Vector/tests/phpunit/structure/BundleSizeTest.php(5)
22:38:27 #0 /workspace/src/vendor/phpunit/phpunit/src/Util/FileLoader.php(66): include_once()
22:38:27 #1 /workspace/src/vendor/phpunit/phpunit/src/Util/FileLoader.php(54): PHPUnit\Util\FileLoader::load(string)
22:38:27 #2 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestSuite.php(384): PHPUnit\Util\FileLoader::checkAndLoad(string)
22:38:27 #3 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestSuite.php(482): PHPUnit\Framework\TestSuite->addTestFile(string)
22:38:27 #4 /workspace/src/tests/phpunit/suites/ExtensionsTestSuite.php(31): PHPUnit\Framework\TestSuite->addTestFiles(array)
22:38:27 #5 /workspace/src/tests/phpunit/suites/ExtensionsTestSuite.php(43): ExtensionsTestSuite->__construct()
22:38:27 #6 [internal function]: ExtensionsTestSuite::suite(string)
22:38:27 #7 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestSuite.php(463): ReflectionMethod->invoke(NULL, string)
22:38:27 #8 /workspace/src/vendor/phpunit/phpunit/src/Util/Configuration.php(1061): PHPUnit\Framework\TestSuite->addTestFile(string)
22:38:27 #9 /workspace/src/vendor/phpunit/phpunit/src/Util/Configuration.php(907): PHPUnit\Util\Configuration->getTestSuite(DOMElement, array)
22:38:27 #10 /workspace/src/vendor/phpunit/phpunit/src/TextUI/Command.php(963): PHPUnit\Util\Configuration->getTestSuiteConfiguration(string)
22:38:27 #11 /workspace/src/vendor/phpunit/phpunit/src/TextUI/Command.php(202): PHPUnit\TextUI\Command->handleArguments(array)
22:38:27 #12 /workspace/src/tests/phpunit/phpunit.php(141): PHPUnit\TextUI\Command->run(array, boolean)
22:38:27 #13 /workspace/src/tests/phpunit/phpunit.php(204): PHPUnitMaintClass->execute()
22:38:27 #14 {main}

https://integration.wikimedia.org/ci/job/mwcore-phpunit-coverage-patch/37662/console

Php code coverage test on core failed, seems to be related to this task.
Backtrace:

22:38:27 [de95d234e4a84fda36953fc6] [no req]   Error: Class 'MediaWiki\Tests\Structure\BundleSizeTest' not found
22:38:27 Backtrace:
22:38:27 from /workspace/src/skins/Vector/tests/phpunit/structure/BundleSizeTest.php(5)
22:38:27 #0 /workspace/src/vendor/phpunit/phpunit/src/Util/FileLoader.php(66): include_once()
22:38:27 #1 /workspace/src/vendor/phpunit/phpunit/src/Util/FileLoader.php(54): PHPUnit\Util\FileLoader::load(string)
22:38:27 #2 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestSuite.php(384): PHPUnit\Util\FileLoader::checkAndLoad(string)
22:38:27 #3 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestSuite.php(482): PHPUnit\Framework\TestSuite->addTestFile(string)
22:38:27 #4 /workspace/src/tests/phpunit/suites/ExtensionsTestSuite.php(31): PHPUnit\Framework\TestSuite->addTestFiles(array)
22:38:27 #5 /workspace/src/tests/phpunit/suites/ExtensionsTestSuite.php(43): ExtensionsTestSuite->__construct()
22:38:27 #6 [internal function]: ExtensionsTestSuite::suite(string)
22:38:27 #7 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestSuite.php(463): ReflectionMethod->invoke(NULL, string)
22:38:27 #8 /workspace/src/vendor/phpunit/phpunit/src/Util/Configuration.php(1061): PHPUnit\Framework\TestSuite->addTestFile(string)
22:38:27 #9 /workspace/src/vendor/phpunit/phpunit/src/Util/Configuration.php(907): PHPUnit\Util\Configuration->getTestSuite(DOMElement, array)
22:38:27 #10 /workspace/src/vendor/phpunit/phpunit/src/TextUI/Command.php(963): PHPUnit\Util\Configuration->getTestSuiteConfiguration(string)
22:38:27 #11 /workspace/src/vendor/phpunit/phpunit/src/TextUI/Command.php(202): PHPUnit\TextUI\Command->handleArguments(array)
22:38:27 #12 /workspace/src/tests/phpunit/phpunit.php(141): PHPUnit\TextUI\Command->run(array, boolean)
22:38:27 #13 /workspace/src/tests/phpunit/phpunit.php(204): PHPUnitMaintClass->execute()
22:38:27 #14 {main}

https://integration.wikimedia.org/ci/job/mwcore-phpunit-coverage-patch/37662/console

Seems to work now https://integration.wikimedia.org/ci/job/mwcore-phpunit-coverage-patch/37763/console

Note that https://integration.wikimedia.org/ci/job/mwcore-phpunit-coverage-patch/37662/console was triggered by patch number 743658,13, and it doesn't look like patch 13 had been rebased, so the Vector test which depended on a newer version of core wasn't able to execute.

I think there's nothing left to do here.