Page MenuHomePhabricator

PHPUnit data providers should be simple static functions that return plain data
Open, Needs TriagePublic

Assigned To
None
Authored By
tstarling
Mar 23 2023, 10:16 AM
Referenced Files
None
Tokens
"Love" token, awarded by kostajh."Mountain of Wealth" token, awarded by DAlangi_WMF."Mountain of Wealth" token, awarded by Mainframe98.

Description

Non-static data providers are executed with code similar to:

$data = ( new $testClass )->$provider();

From this we can derive the drawbacks:

  • The test case constructor is executed unnecessarily, adding to provider execution time. Note that the time taken to run a few test cases with --filter is especially sensitive to data provider execution time, since all data providers need to be executed in order to get the list to filter.
  • The object state is immediately thrown away. This can be unexpected to developers, who try to use $this for caching, or to share state between providers, or between providers and test cases.

Reviewing existing non-static data providers, I noticed the following rationales, which I consider to be weak:

  • A non-static helper is needed. Perhaps the helper is shared between the provider and the test. But usually this means the data provider is doing too much. Most of the logic and execution time for the test should be in the test itself, not in the provider.
  • The mock builder is needed. But MockBuilder takes a TestCase as a required parameter because mocks have assertions attached to them. Mocks built by the provider will be attached to the fake test case, and certain kinds of assertions will not be executed. Also, mock construction is heavyweight, implemented with eval("class ..."), so for performance reasons it should not be done in the provider.

Some providers return objects, not just plain data. This has the following problems:

  • Object construction or factories, especially Title or User construction, may access services, which is unsafe to do so early.
  • The data provided by tests needs to be formatted in a human-readable way when the test fails.
  • Object construction is often slow.

So I think that, with very few exceptions, PHPUnit data providers should be simple and fast static functions that return plain data. The test, not the provider, should be responsible for converting the data to input suitable for the method under test.

PHPUnit 10 decided to deprecate non-static data providers end of 2022 for probably exactly these reasons.

Details

SubjectRepoBranchLines +/-
mediawiki/coremaster+32 -0
mediawiki/extensions/WikispeechREL1_39+12 -12
mediawiki/extensions/ProofreadPagemaster+10 -6
mediawiki/coremaster+97 -75
mediawiki/extensions/ParserFunctionsmaster+2 -2
mediawiki/extensions/MediaUploadermaster+17 -17
mediawiki/extensions/CollaborationKitmaster+6 -6
mediawiki/extensions/LDAPAuthorizationmaster+10 -10
mediawiki/extensions/ContentStabilizationmaster+2 -2
mediawiki/extensions/ContentDropletsmaster+4 -4
mediawiki/extensions/GoogleLoginmaster+2 -2
mediawiki/extensions/RegexBlockmaster+1 -1
mediawiki/extensions/TEImaster+14 -14
mediawiki/extensions/WikibaseQualityConstraintsmaster+101 -101
mediawiki/extensions/WikiLambdamaster+165 -165
mediawiki/extensions/NSFileRepomaster+1 -1
mediawiki/extensions/NSFileRepoREL1_39+1 -1
mediawiki/extensions/TemplateStylesmaster+1 -1
mediawiki/extensions/Translatemaster+77 -77
mediawiki/extensions/OAuthmaster+11 -11
mediawiki/extensions/WikibaseLexememaster+363 -363
mediawiki/extensions/Wikibasemaster+866 -866
mediawiki/extensions/PageFormsmaster+95 -95
mediawiki/extensions/Echomaster+57 -73
mediawiki/extensions/ArticleCreationWorkflowmaster+65 -59
mediawiki/extensions/PageCheckoutmaster+1 -1
mediawiki/extensions/EnhancedUploadmaster+1 -1
mediawiki/extensions/LDAPSyncAllmaster+1 -1
mediawiki/extensions/WikimediaBadgesmaster+37 -34
mediawiki/extensions/GrowthExperimentsmaster+120 -120
mediawiki/extensions/CentralNoticemaster+2 -2
mediawiki/extensions/WikibaseLexemeCirrusSearchmaster+5 -5
mediawiki/extensions/MachineVisionmaster+8 -15
mediawiki/skins/BlueSpiceDiscoverymaster+3 -3
mediawiki/extensions/MenuEditormaster+5 -5
mediawiki/extensions/WikibaseManifestmaster+6 -6
mediawiki/extensions/DonationInterfacemaster+13 -13
mediawiki/extensions/ProofreadPagemaster+52 -52
mediawiki/extensions/ArrayFunctionsmaster+1 -1
mediawiki/extensions/CampaignEventsmaster+73 -73
mediawiki/skins/TuleapSkinmaster+1 -1
mediawiki/extensions/Cargomaster+19 -19
mediawiki/extensions/ContentTransfermaster+1 -1
mediawiki/extensions/Link_Attributesmaster+1 -1
mediawiki/extensions/Workflowsmaster+15 -24
mediawiki/extensions/CommentStreamsmaster+1 -1
mediawiki/extensions/CreatePageUwmaster+2 -2
mediawiki/extensions/CookieWarningmaster+2 -2
mediawiki/extensions/CreateRedirectmaster+1 -1
mediawiki/extensions/EmailAuthorizationmaster+2 -2
mediawiki/extensions/DrawioEditormaster+2 -2
mediawiki/extensions/SimilarEditorsmaster+1 -1
mediawiki/extensions/SecurityApimaster+2 -2
mediawiki/extensions/UseResourcemaster+2 -2
mediawiki/extensions/Wikispeechmaster+12 -12
mediawiki/extensions/WikispeechSpeechDataCollectormaster+9 -9
mediawiki/extensions/ExternalDatamaster+14 -14
mediawiki/extensions/CategoryExplorermaster+1 -1
mediawiki/extensions/LDAPGroupsmaster+2 -2
mediawiki/extensions/Lingomaster+10 -10
mediawiki/extensions/NotesLinkmaster+1 -1
mediawiki/extensions/MobileFrontendContentProvidermaster+1 -1
mediawiki/extensions/MathSearchmaster+1 -1
mediawiki/extensions/PhpTagsMapsmaster+1 -1
mediawiki/extensions/SimpleSAMLphpmaster+2 -2
mediawiki/extensions/TitleIconmaster+1 -1
mediawiki/extensions/TuleapWikiFarmmaster+2 -2
mediawiki/extensions/TranslateSvgmaster+7 -7
mediawiki/extensions/UnlinkedWikibasemaster+1 -1
mediawiki/extensions/WikibaseMediaInfomaster+33 -33
mediawiki/extensions/EntitySchemamaster+30 -30
mediawiki/extensions/EventLoggingmaster+6 -6
mediawiki/extensions/GeoDatamaster+29 -29
mediawiki/extensions/AbuseFiltermaster+101 -101
mediawiki/extensions/Mathmaster+30 -25
mediawiki/extensions/CheckUsermaster+102 -102
mediawiki/extensions/Thanksmaster+55 -62
mediawiki/extensions/PageImagesmaster+20 -15
mediawiki/extensions/CirrusSearchmaster+134 -134
mediawiki/extensions/CommonsMetadatamaster+12 -15
mediawiki/extensions/WikibaseCirrusSearchmaster+22 -22
mediawiki/extensions/VisualEditormaster+13 -13
mediawiki/extensions/VipsScalermaster+10 -10
mediawiki/skins/WikimediaApiPortalmaster+4 -4
mediawiki/extensions/FileImportermaster+93 -92
mediawiki/extensions/EventBusmaster+14 -14
mediawiki/extensions/Wikidata.orgmaster+1 -1
mediawiki/extensions/Scribuntomaster+4 -4
mediawiki/extensions/ReadingListsmaster+20 -20
mediawiki/extensions/Flowmaster+27 -27
mediawiki/extensions/IPInfomaster+23 -23
mediawiki/extensions/LoginNotifymaster+4 -4
mediawiki/extensions/WikimediaEventsmaster+214 -234
mediawiki/extensions/MobileFrontendmaster+160 -160
mediawiki/extensions/CentralAuthmaster+12 -12
mediawiki/extensions/MassMessagemaster+31 -26
mediawiki/skins/Vectormaster+25 -25
mediawiki/extensions/MediaModerationmaster+4 -7
mediawiki/extensions/Phonosmaster+3 -3
mediawiki/extensions/GlobalBlockingmaster+2 -2
mediawiki/extensions/DiscussionToolsmaster+22 -22
mediawiki/extensions/GlobalPreferencesmaster+4 -4
mediawiki/extensions/TwoColConflictmaster+31 -31
mediawiki/extensions/CategoryTreemaster+1 -1
mediawiki/extensions/ContentTranslationmaster+7 -7
mediawiki/extensions/ChessBrowsermaster+4 -4
mediawiki/extensions/ArticlePlaceholdermaster+4 -4
mediawiki/extensions/QuickSurveysmaster+4 -4
mediawiki/extensions/LocalisationUpdatemaster+2 -2
mediawiki/extensions/SecurePollmaster+3 -3
mediawiki/extensions/PageViewInfomaster+2 -2
mediawiki/extensions/TimedMediaHandlermaster+8 -8
mediawiki/extensions/Cognatemaster+3 -3
mediawiki/extensions/PropertySuggestermaster+8 -8
mediawiki/extensions/TextExtractsmaster+6 -6
mediawiki/extensions/Wikistoriesmaster+1 -1
mediawiki/extensions/UniversalLanguageSelectormaster+1 -1
mediawiki/extensions/WikidataPageBannermaster+6 -6
mediawiki/extensions/TrustedXFFmaster+1 -1
mediawiki/extensions/Wikisourcemaster+17 -17
mediawiki/extensions/TranslationNotificationsmaster+1 -1
mediawiki/extensions/TitleBlacklistmaster+1 -1
mediawiki/extensions/SpamBlacklistmaster+2 -2
mediawiki/extensions/UploadWizardmaster+2 -2
mediawiki/extensions/TemplateDatamaster+7 -7
mediawiki/extensions/ORESmaster+16 -16
mediawiki/extensions/JsonConfigmaster+24 -23
mediawiki/extensions/Kartographermaster+22 -22
mediawiki/extensions/SecureLinkFixermaster+2 -2
mediawiki/extensions/OAuthRateLimitermaster+5 -5
mediawiki/extensions/Quizmaster+8 -8
mediawiki/extensions/Babelmaster+5 -5
mediawiki/extensions/Disambiguatormaster+1 -1
mediawiki/extensions/FlaggedRevsmaster+4 -4
mediawiki/extensions/InterwikiSortingmaster+5 -5
mediawiki/extensions/GlobalWatchlistmaster+3 -3
mediawiki/extensions/GlobalCssJsmaster+1 -2
mediawiki/extensions/GuidedTourmaster+2 -2
mediawiki/extensions/EventStreamConfigmaster+3 -3
mediawiki/extensions/ConfirmEditmaster+26 -26
mediawiki/extensions/Collectionmaster+41 -41
mediawiki/extensions/ExternalGuidancemaster+1 -1
mediawiki/extensions/Citemaster+18 -18
mediawiki/extensions/Popupsmaster+8 -8
mediawiki/extensions/FileExportermaster+1 -1
mediawiki/extensions/AntiSpoofmaster+4 -4
mediawiki/extensions/AdvancedSearchmaster+1 -1
mediawiki/skins/MinervaNeuemaster+1 -1
mediawiki/coremaster+95 -95
mediawiki/coremaster+78 -25
mediawiki/coremaster+694 -694
Show related patches Customize query in gerrit

Related Objects

StatusSubtypeAssignedTask
StalledNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
OpenNone
Resolvedthiemowmde
OpenNone
OpenNone
ResolvedLucas_Werkmeister_WMDE
OpenNone
OpenNone
OpenNone
Resolvedtstarling
OpenNone
ResolvedDreamy_Jazz
ResolvedDreamy_Jazz
ResolvedPhysikerwelt
ResolvedTgr
OpenNone
OpenNone
ResolvedNone
OpenNone
OpenNone

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 921716 merged by jenkins-bot:

[mediawiki/extensions/CreatePageUw@master] tests: Make PHPUnit data providers static

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

Change 921711 merged by jenkins-bot:

[mediawiki/extensions/CommentStreams@master] tests: Make PHPUnit data providers static

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

Change 921751 merged by jenkins-bot:

[mediawiki/extensions/Workflows@master] tests: Make PHPUnit data providers static

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

Change 921728 merged by jenkins-bot:

[mediawiki/extensions/Link_Attributes@master] tests: Make PHPUnit data providers static

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

Change 921714 merged by jenkins-bot:

[mediawiki/extensions/ContentTransfer@master] tests: Make PHPUnit data providers static

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

Change 921709 merged by jenkins-bot:

[mediawiki/extensions/Cargo@master] tests: Make PHPUnit data providers static

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

Change 921698 merged by jenkins-bot:

[mediawiki/skins/TuleapSkin@master] tests: Make some PHPUnit data providers static

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

Change 921649 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] tests: Make some PHPUnit data providers static

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

Change 921644 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] tests: Make some PHPUnit data providers static

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

Change 921743 merged by jenkins-bot:

[mediawiki/extensions/WikibaseManifest@master] tests: Make PHPUnit data providers static

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

Change 921651 merged by jenkins-bot:

[mediawiki/extensions/DonationInterface@master] tests: Make PHPUnit data providers static

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

Change 921590 merged by jenkins-bot:

[mediawiki/extensions/WikibaseLexemeCirrusSearch@master] tests: Make some PHPUnit data providers static

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

Change 921703 merged by jenkins-bot:

[mediawiki/extensions/ArrayFunctions@master] tests: Make PHPUnit data providers static

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

Change 921605 merged by jenkins-bot:

[mediawiki/extensions/MachineVision@master] tests: Make PHPUnit data providers static

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

Change 921648 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] tests: Make some PHPUnit data providers static

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

Change 921489 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@master] tests: Make PHPUnit data providers static

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

Change 921604 merged by jenkins-bot:

[mediawiki/extensions/WikimediaBadges@master] tests: Make PHPUnit data providers static

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

Change 921732 merged by Umherirrender:

[mediawiki/extensions/MenuEditor@master] tests: Make PHPUnit data providers static

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

Change 921697 merged by Umherirrender:

[mediawiki/skins/BlueSpiceDiscovery@master] tests: Make some PHPUnit data providers static

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

Change 921615 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] tests: Make PHPUnit data providers static

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

Change 921621 merged by jenkins-bot:

[mediawiki/extensions/ArticleCreationWorkflow@master] tests: Make PHPUnit data providers static

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

Change 921630 merged by jenkins-bot:

[mediawiki/extensions/WikibaseLexeme@master] tests: Make some PHPUnit data providers static

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

Change 921737 merged by jenkins-bot:

[mediawiki/extensions/PageForms@master] tests: Make PHPUnit data providers static

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

Change 921633 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] tests: Make some PHPUnit data providers static

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

Change 921726 merged by jenkins-bot:

[mediawiki/extensions/LDAPSyncAll@master] tests: Make PHPUnit data providers static

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

Change 921721 merged by jenkins-bot:

[mediawiki/extensions/EnhancedUpload@master] tests: Make PHPUnit data providers static

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

Change 921736 merged by jenkins-bot:

[mediawiki/extensions/PageCheckout@master] tests: Make PHPUnit data providers static

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

Change 921642 merged by jenkins-bot:

[mediawiki/extensions/OAuth@master] tests: Make some PHPUnit data providers static

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

Change 921595 merged by jenkins-bot:

[mediawiki/extensions/Translate@master] tests: Make PHPUnit data providers static

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

Change 921522 merged by jenkins-bot:

[mediawiki/extensions/TemplateStyles@master] tests: Make PHPUnit data providers static

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

Change 921735 merged by Robert Vogel:

[mediawiki/extensions/NSFileRepo@master] tests: Make PHPUnit data providers static

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

Change 922400 had a related patch set uploaded (by Robert Vogel; author: Umherirrender):

[mediawiki/extensions/NSFileRepo@REL1_39] tests: Make PHPUnit data providers static

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

Change 922400 merged by Robert Vogel:

[mediawiki/extensions/NSFileRepo@REL1_39] tests: Make PHPUnit data providers static

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

Change 921634 merged by jenkins-bot:

[mediawiki/extensions/WikiLambda@master] tests: Make PHPUnit data providers static

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

Change 921744 merged by Umherirrender:

[mediawiki/extensions/TEI@master] tests: Make PHPUnit data providers static

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

Change 921739 merged by Umherirrender:

[mediawiki/extensions/RegexBlock@master] tests: Make PHPUnit data providers static

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

Change 921723 merged by Umherirrender:

[mediawiki/extensions/GoogleLogin@master] tests: Make PHPUnit data providers static

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

Change 921712 merged by Umherirrender:

[mediawiki/extensions/ContentDroplets@master] tests: Make PHPUnit data providers static

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

Change 921713 merged by Umherirrender:

[mediawiki/extensions/ContentStabilization@master] tests: Make PHPUnit data providers static

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

Change 921710 merged by Umherirrender:

[mediawiki/extensions/CollaborationKit@master] tests: Make PHPUnit data providers static

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

Change 921724 merged by Umherirrender:

[mediawiki/extensions/LDAPAuthorization@master] tests: Make PHPUnit data providers static

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

Change 921731 merged by Umherirrender:

[mediawiki/extensions/MediaUploader@master] tests: Make PHPUnit data providers static

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

Change 921509 merged by jenkins-bot:

[mediawiki/extensions/ParserFunctions@master] tests: Make PHPUnit data providers static

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

Change 931703 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] DifferenceEngineTest: improve data providers

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

Change 931703 merged by jenkins-bot:

[mediawiki/core@master] DifferenceEngineTest: improve data providers

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

Change 942703 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/extensions/ProofreadPage@master] tests: Simplify provider in WikitextLinksExtractorTest

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

Change 942703 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] tests: Simplify provider in WikitextLinksExtractorTest

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

Change 964904 had a related patch set uploaded (by Sebastian Berlin (WMSE); author: Umherirrender):

[mediawiki/extensions/Wikispeech@REL1_39] tests: Make PHPUnit data providers static

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

Change 964904 merged by jenkins-bot:

[mediawiki/extensions/Wikispeech@REL1_39] tests: Make PHPUnit data providers static

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

Change #1050067 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] tests: Make data providers static in PermissionCheckerTest, part 1

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

Change #1050073 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] tests: Make remaining data providers static in PermissionCheckerTest

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