Page MenuHomePhabricator

Add a PHPUnit group to skip test on gated CI runs
Closed, ResolvedPublic

Description

Create a PHPUnit group in mediawiki/core, add a new testsuite that would exclude the group in gated runs and then switch the wmf-quibble jobs to use that new suite.

Background
Several extensions[1] that should be added to the extension-gated CI runs have very time consuming tests that would increase run time tremendously. Therefore these extensions are not added to the gated runs, potentially risking that things break unnoticed.

See also the discussion roughly starting here T125050#5120646

[1] MediaWiki-extensions-Scribunto, CirrusSearch or the FileImporter from Move-Files-To-Commons

Related Objects

Event Timeline

hashar awarded a token.Jun 5 2019, 5:18 PM

Is this waiting for anything other than naming to be settled?

Can we use the standard @small/@medium/@large tags (https://phpunit.de/manual/6.5/en/appendixes.annotations.html) that PHPUnit already supports?

Can we use the standard @small/@medium/@large tags (https://phpunit.de/manual/6.5/en/appendixes.annotations.html) that PHPUnit already supports?

I don't think (ab)using the semantics of those would be a great idea. Some isolated unit tests might be very fast but still unnecessary to run repeatedly, and full-stack system integration tests despite being slow are the value we get from testing code together…

hashar added a comment.Jun 5 2019, 6:43 PM

...

  • Then there are the 1000+ tests in tests/phpunit/engines/LuaCommon, which are run for both LuaSandbox and LuaStandalone to ensure things work under both environments.
    • The run for LuaSandbox takes around 34 seconds on my laptop, and the run for LuaStandalone takes around 163 seconds.

...

And some other similar candidates:

CirrusSearch\SearcherTest::testSearchText
MathCoverageTest::testCoverage

Taking Wikibase as an example, CI eventually injects CirrusSearch, Math and Scribunto (among others). Since we run tests/phpunit/phpunit.php --testsuite extensions then every tests get run which is not so nice. In T125050 we started talking about adding a way to flag tests as being standalone / only for a given extension. Or in other term, that they are not integration tests between extensions.

Given a patch send to Wikibase, we certainly want to run all of its tests. I believe that can be done by simply passing the extension path as an argument: tests/phpunit/phpunit.php --testsuite extensions -- extensions/Wikibase. I am not sure whether the structure or parser tests would be properly included though.

Then we will want to find out whether we want other extensions tests to be:

blacklisted

Exclude tests marked as being standalone / not integration tests. Eg: tests/phpunit/phpunit.php --testsuite extensions --exclude-group standalone (or whatever name we settle on).

There is a risk a test is actually depending on some other extension and thus might suddenly break when the other extension is modified. But I guess we can trust developers best judgment there. All other tests are still run so we get ample coverage and prevent breakages.

whitelist

Run only tests explicitly flagged as being integration tests. Eg tests/phpunit/phpunit.php --testsuite extensions --group integration. That would by far the fastest since no more tests would be run. But I don't think we are well equipped to find out whether a test is actually not relying on any other code. So I think it is rather risky. The advantage of whitelisting with @group integration is that we run no tests by default.

A third way would be a mixture of both system. So that potentially a test from extension Wikibase could be flagged as being an integration test for Scribunto. As such it would only be run for patchsets proposed for those two repositories, but not for a patch proposed to another extension which might happen to depend on both Wikibase and Scribunto.

That potentially could be added as @group statement so that in Wikibase, an integration test with Scribunto would use @group Scribunto. Then CI would:

  • on a patch to Wikibase
    • clone its dependencies and reverse dependencies
    • run all Wikibase tests via phpunit.php extensions/Wikibase
    • run all tests marked @group Wikibase or @group integration
  • on a patch to Scribunto
    • clone its dependencies
    • run all Scribunto tests via phpunit.php extensions/Scribunto
    • run all tests marked @group Scribunto or @group integration

Note that Scribunto does not have Wikibase defined as a dependency. But the wmf-quibble-* jobs have a hardcoded list of extensions being cloned in which includes both, thus a patchset to Scribunto would actually get Wikibase tests marked @group Scribunto.

For the other CI jobs (the one that get dependencies injected), we would need to inject the reverse dependencies. Right now Scribunto only have SyntaxHighlight_GeSHi injected and thus those jobs would not catch a breakage of Wikibase (but the wmf-quible job above do).


I am more than happy to see people jumping on this issue. I have been struggling with it with no good way to even write down a proper problem statement :-\ My status quo so far has been to just run everything.

greg removed a subscriber: greg.Jun 6 2019, 6:41 PM

OK, let's move this forward with a decision; the "blacklist" model, for simplicity:

  • we'll create a new group, "standalone";
  • we'll make quibble run phpunit with --group standalone for all patches to that repo, and --exclude-group standalone for all others; and
  • we'll have authors manually move tests to the new group, starting with the big ones in Scribunto, CirrusSearch, and Math that have already been discussed, with a plea to wikitech-l for people to find and add more.

Sound OK?

hashar added a comment.Mar 6 2020, 5:52 PM

OK, let's move this forward with a decision; the "blacklist" model, for simplicity:

  • we'll create a new group, "standalone";
  • we'll make quibble run phpunit with --group standalone for all patches to that repo, and --exclude-group standalone for all others; and
  • we'll have authors manually move tests to the new group, starting with the big ones in Scribunto, CirrusSearch, and Math that have already been discussed, with a plea to wikitech-l for people to find and add more.

Sound OK?

YES :)

The devil is to find out how to run that --group standalone. That is probably something that should be coded in Quibble, eg a phpunit-standalone stage.

Change 577638 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/quibble@master] Add new command, phpunit-standalone, for phpunit --group Standalone

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

Jdforrester-WMF triaged this task as Medium priority.Mar 18 2020, 5:13 PM
DannyS712 updated the task description. (Show Details)Mar 20 2020, 6:21 PM

Change 577638 merged by jenkins-bot:
[integration/quibble@master] Add phpunit-standalone, for phpunit --group Standalone

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

Change 587580 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/quibble@master] Release Quibble 0.0.41

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

Change 587580 merged by jenkins-bot:
[integration/quibble@master] Release Quibble 0.0.41

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

Change 587588 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] dockerfiles: [quibble-stretch] Install quibble 0.0.41 and cascade.

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

Change 587589 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] jjb: [quibble*] Switch to quibble 0.0.41 images

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

Change 587592 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] jjb: Provide jobs for running "standalone" phpunit tests only

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

Change 587593 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] layout: [mediawiki/extensions/Scribunto] Run standalone path too

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

Change 587588 merged by jenkins-bot:
[integration/config@master] dockerfiles: [quibble-stretch] Install quibble 0.0.41 and cascade.

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

Change 587589 merged by jenkins-bot:
[integration/config@master] jjb: [quibble*] Switch to quibble 0.0.41 images

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

Change 587592 merged by jenkins-bot:
[integration/config@master] jjb: Provide jobs for running "standalone" phpunit tests only

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

Change 587599 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] layout: [mediawiki/extensions/Scribunto] Standalone, experimentally

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

Change 587599 merged by jenkins-bot:
[integration/config@master] layout: [mediawiki/extensions/Scribunto] Standalone, experimentally

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

Mentioned in SAL (#wikimedia-releng) [2020-04-08T21:18:42Z] <James_F> layout: [mediawiki/extensions/Scribunto] Standalone, experimentally T225068

Change 587593 merged by jenkins-bot:
[integration/config@master] layout: [mediawiki/extensions/Scribunto] Run standalone always

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

Change 587600 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/Scribunto@master] tests: Mark LuaStandalone tests as @group Standalone

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

Mentioned in SAL (#wikimedia-releng) [2020-04-08T21:28:37Z] <James_F> layout: [mediawiki/extensions/Scribunto] Standalone, always T225068

Jdforrester-WMF closed this task as Resolved.Apr 8 2020, 9:55 PM

This is now live (for master). Any tests with @Standalone won't be run for any repos, except those with special CI config, which will run them as a different job. So far the only repo configured this way is Scribunto. Follow-up in tasks under T249674: Have all Wikimedia production extensions and skins in the CI gate, please.

Aklapper removed a subscriber: Anomie.Fri, Oct 16, 5:41 PM