Page MenuHomePhabricator

Preparation for the PHPUnit 9 upgrade
Open, MediumPublic

Description

Yesterday, we've finally upgraded to PHPUnit 8 (T192167). However, PHPUnit 9 is scheduled for release on Feb 7th, which means that we can start thinking about it. IMHO, we shouldn't rush the upgrade (given that various repos are probably still incompatible with PHPUnit 8). However, I find it wise to start preparing now, instead of letting tech debt accumulate.

This is especially true because PHPUnit 9 will have several compat breaks (source). Most notably:

  • MockBuilder::setMethods will be hard-deprecated (T278010);
  • Assert::assertRegExp is hard-deprecated in favour of assertMatchesRegularExpression(), which however was also introduced in PHPUnit 9
  • The TestListener interface will be removed in favour of hooks.
  • ResultPrinter will become an interface. We subclass the current ResultPrinter to augment logs (the "logs generated by test case" lines); That should extend DefaultResultPrinter (introduced in PHPUnit 9.0.0) instead.
  • The methods that were hard-deprecated in PHPUnit 8 (assertInternalType, assertAttribute*, assertArraySubset, ...) will be removed; shouldn't be a problem, related PHPCS sniffs have been in place for many months
  • It will not support PHP 7.2. As such, we cannot upgrade until T261872 is done.
  • Additional minor fixes:
    • Several traits declare createMock as abstract but the signature will change in PHPUnit 9 (string typehint added); the traits should probably not declare the method at all, and use @method or an if+throw. List: MockTitleTrait, MockAuthorityTrait, MockServiceDependenciesTrait (also fail(), but the signature is the same), HandlerTestTrait
    • MockHttpTrait redeclares getMockBuilder(). Same as above.
    • Our XML schema is outdated (--migrate-configuration to update it)
    • @coversNothing should be added to MediaWikiCoversValidator::testValidCovers
    • ApiTestCase uses $this->contains() which should become $this->containsIdentical

Optional things which might (or will) help with future upgrades, e.g. PHPUnit 10:

  • Possibly avoid a custom printer
  • Removing MediaWikiPHPUnitCommand and our custom entry point (T90875)
  • Removing our PHPUnit extensions: MediaWikiHooksPHPUnitExtension (blocked on T278011), MediaWikiLoggerPHPUnitExtension (used in conjunction with the ResultPrinter)

Related Objects

StatusSubtypeAssignedTask
StalledNone
OpenNone
Resolvedaaron
ResolvedMholloway
ResolvedAmmarpad
ResolvedReedy
ResolvedReedy
ResolvedUmherirrender
OpenNone
ResolvedLucas_Werkmeister_WMDE
StalledNone
StalledNone
OpenNone
Resolvedhashar
ResolvedJdforrester-WMF
ResolvedLadsgroup
ResolvedMoritzMuehlenhoff
Resolvedjijiki
ResolvedMoritzMuehlenhoff
ResolvedTrizek-WMF
ResolvedDzahn
ResolvedGilles
StalledDzahn
ResolvedRequestPapaul
Resolvedjijiki
DeclinedNone
ResolvedDzahn
OpenNone
ResolvedDzahn
ResolvedPapaul
ResolvedCmjohnson
Opendancy
OpenNone
ResolvedRequestCmjohnson
ResolvedRequestPapaul
ResolvedAndrew
ResolvedArielGlenn
ResolvedDzahn
ResolvedLegoktm
OpenNone
ResolvedPapaul
ResolvedDzahn
DeclinedGilles
ResolvedVolans
ResolvedDzahn
OpenNone

Event Timeline

Change 566829 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/core@master] Use PHPUnit hooks for augmented logs

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

With pleasure! This task should stay at low priority; the most important actionable is creating new sniffs for deprecated methods as soon as we can, so we don't have to rush the LibUp upgrades later.

Change 583591 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Don’t use deprecated assertion function

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

Change 583591 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Don’t use deprecated assertion function

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

Change 585438 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/core@master] Replace deprecated PHPUnit method in test

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

Change 585438 merged by jenkins-bot:
[mediawiki/core@master] Replace deprecated PHPUnit method in test

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

Change 566829 merged by jenkins-bot:
[mediawiki/core@master] Use PHPUnit hooks for augmented logs

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

Some words about the phpunit 9 - https://thephp.cc/news/2020/02/migrating-to-phpunit-9

I have created a sub task for the usage of expectDeprecation/expectNotice/expectWarning/expectError
There are only less usages and does not need a sniff.

Using an array to create mocks seems not used on ourer code base and needs no sniff.
https://codesearch.wmcloud.org/search/?q=(getMockBuilder%7CcreateMock%7CcreateConfiguredMock%7CcreatePartialMock)%5Cs*%5C(%5Cs*%5C%5B&i=nope&files=&repos=

The rest from that essay is already addressed.

Change 673704 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/core@master] phpunit: Remove MediaWikiPHPUnitTestListener

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

Daimona raised the priority of this task from Low to Medium.Mar 20 2021, 2:54 PM

There are lots of things to do, so I think this shouldn't be low priority, even if the upgrade itself is low priority.

Change 673705 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/core@master] Deprecate PHPUnit hooks

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

Change 673704 merged by jenkins-bot:
[mediawiki/core@master] phpunit: Remove MediaWikiPHPUnitTestListener

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

Change 674285 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/core@master] phpunit: Don't redefine methods in traits

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

We can allow 8.5|9.5 so that a repo can simultaneously work on PHP 7.2 and PHP 8.0; that's what I've been doing for various libraries as I'm going around. That was this work isn't blocked by dropping 7.2 support.

Change 674285 merged by jenkins-bot:
[mediawiki/core@master] phpunit: Don't redefine methods in traits

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

We can allow 8.5|9.5 so that a repo can simultaneously work on PHP 7.2 and PHP 8.0; that's what I've been doing for various libraries as I'm going around. That was this work isn't blocked by dropping 7.2 support.

Yes, this is indeed a good idea. The only caveat is that we're going to need a couple of BC hacks for renamed classes, but should be easily doable overall. Tentatively sending a patch now (although T278010 should still be resolved first).

Change 674851 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):
[mediawiki/core@master] [WIP] Allow PHPUnit 9

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