The @dataProvider annotation should be a static function, check and make data provider in the extension static and adjust the usages (More infos at T332865).
Some abstract provider function are also used in other extensions and need some compatibility code.
Initial work was done in e875e0b021d4d727adc57f89f1890eb789dae758
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Stalled | None | T255792 Quibble runs core:unit tests twice! | |||
| Open | None | T328919 Upgrade to PHPUnit 10 | |||
| Open | None | T225730 Reduce runtime of MW shared gate Jenkins jobs to 5 min | |||
| Open | None | T332865 PHPUnit data providers should be simple static functions that return plain data | |||
| Open | None | T371467 [GENERAL] Make PHPUnit dataProvider static in Wikibase-related extensions | |||
| Resolved | karapayneWMDE | T337154 Make PHPUnit dataProvider static in Wikibase tests | |||
| Resolved | AudreyPenven_WMDE | T380604 Make provideLuaData static | |||
| Resolved | AudreyPenven_WMDE | T380605 Make EntityMetaTagsCreatorTestCase's provideTestGetMetaTags dataProvider static |
Change 927136 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] MediawikiEditEntityTest: Improve data providers
Change 927136 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] MediawikiEditEntityTest: Improve data providers
Change #1082197 had a related patch set uploaded (by Audrey Penven; author: Audrey Penven):
[mediawiki/extensions/Wikibase@master] [WIP] Make dataProvider function static
Change #1082819 had a related patch set uploaded (by Audrey Penven; author: Audrey Penven):
[mediawiki/extensions/Wikibase@master] Make phpunit dataProviders static, part 1
Change #1082197 abandoned by Audrey Penven:
[mediawiki/extensions/Wikibase@master] [WIP] Make dataProvider function static
Reason:
this commit is too large. breaking it into several smaller ones
Change #1082821 had a related patch set uploaded (by Audrey Penven; author: Audrey Penven):
[mediawiki/extensions/Wikibase@master] Make phpunit dataProviders static, part 2
Change #1082822 had a related patch set uploaded (by Audrey Penven; author: Audrey Penven):
[mediawiki/extensions/Wikibase@master] Make phpunit dataProviders static, part 3
Change #1082824 had a related patch set uploaded (by Audrey Penven; author: Audrey Penven):
[mediawiki/extensions/Wikibase@master] Make phpunit dataProviders static, part 4
The 4 patch sets fix the dataProvider functions in 59 files. I thought these were reasonably sized chunks to review, but am open to combining them, or breaking them up further as needed.
I've identified 26 files still remaining, which require more complex solutions. Those are still in progress.
Change #1084182 had a related patch set uploaded (by Audrey Penven; author: Audrey Penven):
[mediawiki/extensions/Wikibase@master] [WIP] make dataProviders static
Change #1082819 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Make phpunit dataProviders static, part 1
Change #1082821 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Make phpunit dataProviders static, part 2
Change #1085439 had a related patch set uploaded (by Audrey Penven; author: Audrey Penven):
[mediawiki/extensions/WikibaseMediaInfo@master] Support making Wikibase dataProviders static
Change #1085588 had a related patch set uploaded (by Audrey Penven; author: Audrey Penven):
[mediawiki/extensions/WikibaseMediaInfo@master] Temporarily skip DeserializationTesters
Change #1085588 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Temporarily skip DeserializationTesters
Change #1082822 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Make phpunit dataProviders static, part 3
Change #1082824 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Make phpunit dataProviders static, part 4
Change #1085439 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Support making Wikibase dataProviders static
Another batch of these is ready for review: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/1084182
After this, there are 5 test files remaining (that I was able to find):
These are abstract classes, or otherwise are used by tests in other extensions. Changing them involves a multi-step process of skipping tests in the other repo, making the functions static in Wikibase, making the function implementations static in the other repos (WikibaseLexeme and WikibaseCirrusSearch) and re-enabling the tests there. I think it makes the most sense to do the commit chains with the smallest set of changes needed, rather than include it in the patchset linked above.
Change #1084182 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Make dataProviders static in tests with mocks
Change #1090877 had a related patch set uploaded (by Audrey Penven; author: Audrey Penven):
[mediawiki/extensions/Wikibase@master] [WIP] dataProvider staticization in EntityProvider
Change #1091780 had a related patch set uploaded (by Audrey Penven; author: Audrey Penven):
[mediawiki/extensions/WikibaseCirrusSearch@master] Skip tests to facilitate making Wikibase dataProviders static
Change #1091781 had a related patch set uploaded (by Audrey Penven; author: Audrey Penven):
[mediawiki/extensions/WikibaseCirrusSearch@master] Re-enable tests, make dataProviders static
Change #1092208 had a related patch set uploaded (by Audrey Penven; author: Audrey Penven):
[mediawiki/extensions/WikibaseLexeme@master] Skip tests to facilitate making Wikibase dataProviders static
Change #1092209 had a related patch set uploaded (by Audrey Penven; author: Audrey Penven):
[mediawiki/extensions/WikibaseLexeme@master] Make dataProviders defined in Wikibase static
The (final?) set of patch sets are ready for review, and involve changes in 3 repos: Wikibase, WikibaseLexeme and WikibaseCirrusSearch
The first 2 skip the affected tests in WikibaseLexeme and WikibaseCirrusSearch:
The 3rd in the chain is the main change, in Wikibase:
The last 2 re-enable the skipped tests, and make affected dataProvider implementations static in WikibaseLexeme and WikibaseCirrusSearch:
In addition to a review of the code itself, double-checking that I got the dependency order correct on these for a smooth rollout would be helpful :)
Change #1091780 merged by jenkins-bot:
[mediawiki/extensions/WikibaseCirrusSearch@master] Skip tests to facilitate making Wikibase dataProviders static
Change #1092208 merged by jenkins-bot:
[mediawiki/extensions/WikibaseLexeme@master] Skip tests to facilitate making Wikibase dataProviders static
Change #1090877 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Make dataProviders static in abstract classes
Change #1092209 merged by jenkins-bot:
[mediawiki/extensions/WikibaseLexeme@master] Re-enable tests, make dataProviders static
Change #1091781 merged by jenkins-bot:
[mediawiki/extensions/WikibaseCirrusSearch@master] Re-enable tests, make dataProviders static
I’m still finding some other non-static data providers with these searches FWIW (and at least one, in NoBadUsageTestBase, seems to be a real data provider and not just a function that randomly has a similar name).
Change #1092268 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Make NoBadUsageTestBase test data providers static
Change #1092268 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Make NoBadUsageTestBase test data providers static
Change #1092302 had a related patch set uploaded (by Audrey Penven; author: Audrey Penven):
[mediawiki/extensions/Wikibase@master] [WIP] make dataProviders static in lib, client, view
For future reference, this what I used to find non-static dataProvider functions. It iterates through all files, extracts the function names from each line with a @dataProvider annotation, and then looks within that file for the function definition. If it doesn't have static in the definition, it gets added to the output array. The "unclear functions" are when a function definition can't be found in that file. All of these cases were of a function being defined in another class.
<?php $path = 'extensions/Wikibase/'; $dirs = [ 'client/', 'data-access/', 'lib/', 'repo/', 'tests/', 'view/' ]; function isStatic( $functionName, $filename ) { $pattern = '/\s*public( static)? function ' . $functionName . '\(\)/'; $functionDefinition = preg_grep( $pattern, file( $filename ) ); if ( sizeOf ( $functionDefinition ) != 1 ) { return "ERROR: " . $functionName . " had " . sizeof( $functionDefinition ) . " matches.\n"; } return str_contains( array_values( $functionDefinition )[0], 'static' ); }; foreach ( $dirs as $dir ) { echo( "\n\n\n++++++++++++++++++++++++++++++++++\n" ); echo( $dir ); echo( "\n++++++++++++++++++++++++++++++++++\n" ); $rdi = new RecursiveDirectoryIterator($path . $dir, RecursiveDirectoryIterator::KEY_AS_PATHNAME); $nonStaticFunctionsByFilename = []; $unclearFunctionsByFilename = []; $doneFilesCount = 0; foreach (new RecursiveIteratorIterator($rdi, RecursiveIteratorIterator::SELF_FIRST) as $file => $info) { if ( str_ends_with( $file, '.php' ) ) { $nonStatic = []; $unclear = []; foreach ( file( $file ) as $line ) { $match = null; if ( preg_match( "/(?<=@dataProvider\s)\s*\w*(?=((\(\))?(\s\*\/)?$))/", $line, $match ) ) { $functionName = trim( $match[0] ); $staticResult = isStatic( $functionName, $file ); if ( is_string( $staticResult ) ) { $unclear[] = $functionName; } elseif ( !$staticResult ) { $nonStatic[] = $functionName; } } } if ( sizeof ( $unclear ) > 0 ) { $unclearFunctionsByFilename[$file] = $unclear; } if ( sizeof ( $nonStatic ) > 0 ) { $nonStaticFunctionsByFilename[$file] = $nonStatic; } if ( sizeof ( $nonStatic ) == 0 && sizeof( $unclear ) == 0) { $doneFilesCount++; } } } if ( sizeof ( $nonStaticFunctionsByFilename ) > 0 ) { echo( "\nNonStatic functions by filename:" ); print_r( $nonStaticFunctionsByFilename ); } if ( sizeof ( $unclearFunctionsByFilename ) > 0 ) { echo( "\nUnclear functions by filename:" ); print_r( $unclearFunctionsByFilename ); } echo sizeof( $nonStaticFunctionsByFilename ) . " files affected\n"; echo sizeof( $unclearFunctionsByFilename ) . " unclear files (usually abstract)\n"; echo $doneFilesCount. " files unaffected\n\n"; }
Change #1092302 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Make dataProviders static in lib, client, view, rest-api
Change #1097447 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Turn “data provider” into test
I found a few more:
$ git grep -e 'public function provide' -e 'public function .*Provider(' client/includes/DataAccess/Scribunto/WikibaseLibrary.php: public function setPropertyOrderProvider( PropertyOrderProvider $propertyOrderProvider ): void { client/includes/Usage/UsageAccumulatorFactory.php: public function newFromParserOutputProvider( ParserOutputProvider $parserOutputProvider ): UsageAccumulator { client/tests/phpunit/integration/includes/DataAccess/Scribunto/WikibaseLibraryInProcessEntityCacheTest.php: public function provideLuaData() { client/tests/phpunit/integration/includes/Hooks/DescriptionProviderHookHandlerTest.php: public function testDescriptionProvider( client/tests/phpunit/unit/includes/Changes/InjectRCRecordsJobTest.php: public function provideMakeJobSpecification(): array { client/tests/phpunit/unit/includes/Changes/InjectRCRecordsJobTest.php: public function provideConstruction(): array { repo/tests/phpunit/includes/Api/GetClaimsTest.php: public function validRequestProvider() { repo/tests/phpunit/includes/Api/RemoveClaimsTest.php: public function itemProvider(): iterable { repo/tests/phpunit/includes/ChangeOp/ChangeOpAliasesTest.php: public function testApplyNoAliasesProvider() { repo/tests/phpunit/includes/ChangeOp/ChangeOpDescriptionTest.php: public function testApplyNoDescriptionsProvider() { repo/tests/phpunit/includes/ChangeOp/ChangeOpLabelTest.php: public function testApplyNoLabelsProvider() { repo/tests/phpunit/includes/Localizer/DispatchingExceptionLocalizerTest.php: public function provideGetExceptionMessageThrowsException() { repo/tests/phpunit/includes/Specials/SpecialEntityDataTest.php: public function testEntityDataFormatProvider(): void { view/tests/phpunit/TermsListViewTest.php: public function testGetTermsListView_noAliasesProvider() {
(git grep '@data[pP]rovider' | grep -v -e '@data[pP]rovider provide' -e '@data[pP]rovider .*Provider\b' shows a few dozen other data providers that are named differently; those will have to be checked manually whether they’re static or not, as it can’t be seen from the @dataProvider line.)
Still TBD AFAIK.
Change #1097447 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Turn “data provider” into test
Change #1098089 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] client: Make InjectRCRecordsJobTest data providers static
I looked through them now and all of those data providers with nonstandard names are static already – Audrey’s script probably detected them :)
So as far as I’m concerned, this is okay to skip tech verification and can directly go to Done once the last Gerrit change and subtasks are done.
Change #1098089 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] client: Make InjectRCRecordsJobTest data providers static
I guess this should go into Parent Task for now, as it’s only waiting for the remaining subtask to be done (but can’t yet be tech-verified for that reason).
Not sure how much more can be tech verified here, to be honest… I think we can probably just close this? (And then we’ll find out during the PHPUnit 10 upgrade if we missed any non-static data providers, or any were added in the meantime even.)