Page MenuHomePhabricator

Replace "db" field usages with new getDb() in MediaWikiIntegrationTestCase
Open, MediumPublic

Description

Many integration tests involve database access. For example, they use:

  • LBFactory::getDatabase()
  • LoadBalancer::getConnection()

All of these methods involve a DBConnRef that properly handles the database domain (e.g. what $db->getDomainId() returns). However, MediaWikiIntegrationTestCase has a member field called "db", which was provided for easy access to a database handles. The problem is that it was created before DBConnRef and just directly points to a Database object (instead of a DBConnRef wrapper). This makes usage of MediaWikiIntegrationTestCase::db idiosyncratic and subject to unexpected database domain changes due to other code getting a connection to a domain that is not the current one.

The field is currently defined using an LB method that is supposed to be internal:

$this->db = $lb->getConnectionInternal( DB_PRIMARY );

This fix for this is to:

  • Fix uses of $this->db that block https://gerrit.wikimedia.org/r/c/mediawiki/core/+/519305
  • Remove use of $this->db from subclasses of MediaWikiIntegrationTestCase that override the field (e.g. $this->db = ... in setUp()) but can just use MediaWikiIntegrationTestCase::getDB() in each test function. For example, DatabaseIntegrationTest currently has $this->db = MediaWikiServices::getInstance()->getConnectionProvider()->getPrimaryDatabase().
  • Remove use of $this->db from subclasses of MediaWikiIntegrationTestCase that rely on the parent class to set the field. They can use MediaWikiIntegrationTestCase::getDB() in each test method instead.

When grepping, note that many classes do harmless things like $this->db = new DatabaseTestHelper(...) and are not subclasses of MediaWikiIntegrationTestCase.

Code search: https://codesearch.wmcloud.org/search/?q=-%3Edb-%3E&files=Test%5C.php%24&excludeFiles=&repos=

Details

SubjectRepoBranchLines +/-
mediawiki/extensions/ReadingListsmaster+17 -6
mediawiki/extensions/FacetedCategorymaster+2 -2
mediawiki/extensions/Flowmaster+8 -7
mediawiki/extensions/LDAPProvidermaster+1 -1
mediawiki/extensions/MissedPagesmaster+2 -2
mediawiki/extensions/HoneyPotmaster+2 -2
mediawiki/extensions/EntitySchemamaster+1 -1
mediawiki/extensions/BlueSpicePageTemplatesREL1_39-4.5.x+1 -1
mediawiki/extensions/BlueSpicePageTemplatesREL1_39+1 -1
mediawiki/extensions/BlueSpicePageTemplatesmaster+1 -1
mediawiki/extensions/PasswordlessLoginmaster+4 -4
mediawiki/extensions/Wikispeechmaster+3 -3
mediawiki/coremaster+2 -0
mediawiki/extensions/Cognatemaster+1 -1
mediawiki/extensions/EntitySchemamaster+1 -1
mediawiki/extensions/WikimediaEventsmaster+1 -1
mediawiki/extensions/PageAssessmentsmaster+1 -1
mediawiki/extensions/Wikispeechmaster+3 -3
mediawiki/extensions/Echomaster+1 -1
mediawiki/extensions/DiscussionToolsmaster+2 -2
mediawiki/extensions/ContentTranslationmaster+3 -3
mediawiki/extensions/TemplateDatamaster+1 -1
mediawiki/coremaster+29 -29
mediawiki/extensions/Wikibasemaster+171 -171
mediawiki/extensions/BlueSpiceFoundationREL1_39+1 -1
mediawiki/extensions/BlueSpiceReminderREL1_39+18 -18
mediawiki/extensions/BlueSpiceReminderREL1_39-4.5.x+18 -18
mediawiki/extensions/BlueSpiceRemindermaster+18 -18
mediawiki/extensions/BlueSpiceFoundationREL1_39-4.5.x+1 -1
mediawiki/extensions/BlueSpiceFoundationmaster+1 -1
mediawiki/extensions/CentralAuthmaster+27 -27
mediawiki/extensions/OAuthRateLimitermaster+2 -2
mediawiki/extensions/PropertySuggestermaster+4 -4
mediawiki/extensions/ReadingListsmaster+45 -45
mediawiki/extensions/AbuseFiltermaster+11 -10
mediawiki/extensions/BlockInactivemaster+13 -13
mediawiki/extensions/FileImportermaster+4 -4
mediawiki/extensions/OATHAuthmaster+2 -2
mediawiki/extensions/GrowthExperimentsmaster+4 -4
mediawiki/extensions/CampaignEventsmaster+19 -19
mediawiki/extensions/MediaModerationmaster+55 -55
mediawiki/coremaster+291 -291
mediawiki/extensions/WikibaseQualityConstraintsmaster+7 -7
mediawiki/extensions/PageTriagemaster+46 -46
mediawiki/extensions/WikiLambdamaster+7 -7
mediawiki/extensions/CheckUsermaster+79 -79
mediawiki/extensions/CheckUsermaster+950 -141
mediawiki/coremaster+36 -34
mediawiki/extensions/AbuseFiltermaster+18 -12
mediawiki/coremaster+17 -1
Show related patches Customize query in gerrit

Event Timeline

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

Change #1050419 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/CentralAuth@master] Tests: Replace "db" with getDb method

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

Change #1050427 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/core@master] Tests: Replace "db" with getDb method

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

Change #1050429 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/OATHAuth@master] Tests: Replace "db" with getDb method

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

Change #1050431 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/GrowthExperiments@master] Tests: Replace "db" method with getDb method

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

Change #1050489 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/BlueSpicePageTemplates@master] Tests:Replace "db" with getDb() method

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

Change #1050431 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Tests: Replace "db" method with getDb method

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

Change #1050429 merged by jenkins-bot:

[mediawiki/extensions/OATHAuth@master] Tests: Replace "db" with getDb method

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

Change #1050803 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/ReadingLists@master] Tests: Replace "db" with getDb method

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

Change #1050804 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/PropertySuggester@master] Tests: Replace "db" with getDb() method

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

Change #1050805 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/OAuthRateLimiter@master] Tests: Replace "db" with getDb method

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

Change #1050806 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/FileImporter@master] Tests: Replace "db" with getDb method

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

Change #1050807 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/BlueSpiceReminder@master] Tests: Replace "db" with getDb() method

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

Change #1050808 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/BlockInactive@master] Tests:Replace "db" with getDb method

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

Change #1050809 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/BlueSpiceFoundation@master] Test:Replace "db" with getDb method

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

Change #1050806 merged by jenkins-bot:

[mediawiki/extensions/FileImporter@master] Tests: Replace "db" with getDb method

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

Change #1050366 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@master] Tests: Repalce "db" with getDb() method

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

Change #1050803 merged by jenkins-bot:

[mediawiki/extensions/ReadingLists@master] Tests: Replace "db" with getDb method

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

Change #1050804 merged by jenkins-bot:

[mediawiki/extensions/PropertySuggester@master] Tests: Replace "db" with getDb() method

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

Change #1050805 merged by jenkins-bot:

[mediawiki/extensions/OAuthRateLimiter@master] Tests: Replace "db" with getDb method

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

Change #1050808 merged by jenkins-bot:

[mediawiki/extensions/BlockInactive@master] Tests:Replace "db" with getDb method

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

Change #1050419 merged by jenkins-bot:

[mediawiki/extensions/CentralAuth@master] Tests: Replace "db" with getDb method

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

Change #1050809 merged by Zoranzoki21:

[mediawiki/extensions/BlueSpiceFoundation@master] Test:Replace "db" with getDb method

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

Change #1050817 had a related patch set uploaded (by Zoranzoki21; author: Wandji collins):

[mediawiki/extensions/BlueSpiceFoundation@REL1_39-4.5.x] Test:Replace "db" with getDb method

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

Change #1050817 merged by Zoranzoki21:

[mediawiki/extensions/BlueSpiceFoundation@REL1_39-4.5.x] Test:Replace "db" with getDb method

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

Change #1050818 had a related patch set uploaded (by Zoranzoki21; author: Wandji collins):

[mediawiki/extensions/BlueSpiceFoundation@REL1_39] Test:Replace "db" with getDb method

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

Change #1050807 merged by Zoranzoki21:

[mediawiki/extensions/BlueSpiceReminder@master] Tests: Replace "db" with getDb() method

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

Change #1050819 had a related patch set uploaded (by Zoranzoki21; author: Wandji collins):

[mediawiki/extensions/BlueSpiceReminder@REL1_39-4.5.x] Tests: Replace "db" with getDb() method

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

Change #1050820 had a related patch set uploaded (by Zoranzoki21; author: Wandji collins):

[mediawiki/extensions/BlueSpiceReminder@REL1_39] Tests: Replace "db" with getDb() method

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

Change #1050819 merged by Zoranzoki21:

[mediawiki/extensions/BlueSpiceReminder@REL1_39-4.5.x] Tests: Replace "db" with getDb() method

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

Change #1050820 abandoned by Zoranzoki21:

[mediawiki/extensions/BlueSpiceReminder@REL1_39] Tests: Replace "db" with getDb() method

Reason:

My mistake, this actually breaks the Bluespice tests.

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

Change #1050818 abandoned by Zoranzoki21:

[mediawiki/extensions/BlueSpiceFoundation@REL1_39] Test:Replace "db" with getDb method

Reason:

My mistake, this actually breaks the Bluespice tests.

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

Change #1043248 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] tests: Use getDb rather than db

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

Change #1050427 merged by jenkins-bot:

[mediawiki/core@master] Tests: Replace "db" with getDb method

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

Change #1052319 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/ContentTranslation@master] Test:Replace "db" with getDb method

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

Change #1052321 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/TemplateData@master] Test: Replace db with getDb method

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

Change #1052324 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/Wikispeech@master] Test:Replace db with getDb() method

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

Change #1052326 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/DiscussionTools@master] Test: Replace db with getDb method

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

Change #1052329 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/Echo@master] Test:Replace db with getDb method

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

Change #1052321 merged by jenkins-bot:

[mediawiki/extensions/TemplateData@master] Test: Replace db with getDb method

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

Change #1052326 merged by jenkins-bot:

[mediawiki/extensions/DiscussionTools@master] Test: Replace db with getDb method

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

Change #1052319 merged by jenkins-bot:

[mediawiki/extensions/ContentTranslation@master] Test:Replace "db" with getDb method

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

Change #1052332 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/EntitySchema@master] Test:Replace db with T316841 method

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

Change #1052329 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] Test:Replace db with getDb method

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

Change #1053280 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/Wikispeech@master] Test: Replacing db method with getDb method

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

Change #1053283 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/EntitySchema@master] Test:Replace db method with getDb method

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

Change #1053280 abandoned by Wandji collins:

[mediawiki/extensions/Wikispeech@master] Test: Replacing db method with getDb method

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

Change #1053285 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/MissedPages@master] Test:Replace db with getDb method

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

Change #1053286 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/LDAPProvider@master] Test:Replace db with getDb method

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

Change #1053287 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/FacetedCategory@master] Test:Replace db with getDb method

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

Some extensions still have master supporting ME < 1.39 so that should be taken into account with these patches (i.e. either update extension.json, or hold off on the change until the extension is ready to drop support for old versions).

Change #1053292 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/PageAssessments@master] Test:Replace db with getDb method

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

Change #1053295 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/WikimediaEvents@master] Test:Replace db with getDb method

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

Change #1053292 merged by jenkins-bot:

[mediawiki/extensions/PageAssessments@master] Test:Replace db with getDb method

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

Change #1053295 merged by jenkins-bot:

[mediawiki/extensions/WikimediaEvents@master] Test:Replace db with getDb method

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

Change #1053322 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/PasswordlessLogin@master] Test:Replace db with getDb method

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

Change #1053283 abandoned by Wandji collins:

[mediawiki/extensions/EntitySchema@master] Test:Replace db method with getDb method

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

Change #1053327 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/Cognate@master] Test:Replace db with getDb method

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

Change #1053329 had a related patch set uploaded (by Wandji collins; author: Wandji collins):

[mediawiki/extensions/HoneyPot@master] Test:Replace db with getDb methods

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

Feels weird to mass refactor these without marking it as deprecated. They'll just creep back into the codebase if it's not marked as deprecated. Any objections to me writing a patch to deprecate protected $db? I would add something like @deprecated since 1.43. Use MediaWikiIntegrationTestCase->getDb() instead. This will eventually be made private.

Some extensions still have master supporting ME < 1.39 so that should be taken into account with these patches (i.e. either update extension.json, or hold off on the change until the extension is ready to drop support for old versions).

Yeah, MediaWikiIntegrationTestCase->getDb() is @since 1.39, so we need to be bumping the extension.json file versions to 1.39 if it is <1.39 when we do this refactor. Does that make sense @Collins?

We should probably try to catch and -1 this in code review. Do we need to go back and fix any?

Change #1053375 had a related patch set uploaded (by Novem Linguae; author: Novem Linguae):

[mediawiki/core@master] MediaWikiIntegrationTestCase: deprecate $db

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

Feels weird to mass refactor these without marking it as deprecated. They'll just creep back into the codebase if it's not marked as deprecated. Any objections to me writing a patch to deprecate protected $db? I would add something like @deprecated since 1.43. Use MediaWikiIntegrationTestCase->getDb() instead. This will eventually be made private.

It's a good idea, the deprecation annotation will have to be for mostly test files (Not all though), Some test files make use of a protected $db reference in the IDatabase.

Some extensions still have master supporting ME < 1.39 so that should be taken into account with these patches (i.e. either update extension.json, or hold off on the change until the extension is ready to drop support for old versions).

Yeah, MediaWikiIntegrationTestCase->getDb() is @since 1.39, so we need to be bumping the extension.json file versions to 1.39 if it is <1.39 when we do this refactor. Does that make sense @Collins?

We should probably try to catch and -1 this in code review. Do we need to go back and fix any?

I just saw the patch, thank you, That will go a long way, and I will lookout for extensions that need the extension.json updated.

Note that extensions with a compatibility policy of master that shouldn't be bumped to 1.39 unless their maintainers agree — they may be wanting to keep compatibility with older versions for a reason.

I think the language bundle is on 1.40, and I don't know of any other repos with that compatibility policy, so hopefully there aren't too many of those that apply here.

The one I noticed was MissedPages. I've not gone through the rest though. You're probably right, there aren't that many.

Is this bulk change supposed to just be for Wikimedia-deployed extensions though? It looks like it's all extensions, so master would be used at least sometimes.

I will pay attention to extension versions and raise concerns if the MediaWiki version supported by the extension has to be changed.

Also, not all the extensions need to be updated, I have noticed a few.

Change #1053327 merged by jenkins-bot:

[mediawiki/extensions/Cognate@master] Test:Replace db with getDb method

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

Change #1053375 merged by jenkins-bot:

[mediawiki/core@master] MediaWikiIntegrationTestCase: deprecate $db

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

Change #1052324 merged by Umherirrender:

[mediawiki/extensions/Wikispeech@master] Test:Replace db with getDb() method

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

Change #1053322 merged by jenkins-bot:

[mediawiki/extensions/PasswordlessLogin@master] Test:Replace db with getDb method

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

Change #1056067 had a related patch set uploaded (by Dvogel hallowelt; author: Wandji collins):

[mediawiki/extensions/BlueSpicePageTemplates@REL1_39] Tests:Replace "db" with getDb() method

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

Change #1056068 had a related patch set uploaded (by Dvogel hallowelt; author: Wandji collins):

[mediawiki/extensions/BlueSpicePageTemplates@REL1_39-4.5.x] Tests:Replace "db" with getDb() method

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

Change #1050489 merged by Zoranzoki21:

[mediawiki/extensions/BlueSpicePageTemplates@master] Tests:Replace "db" with getDb() method

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

Change #1056067 merged by Zoranzoki21:

[mediawiki/extensions/BlueSpicePageTemplates@REL1_39] Tests:Replace "db" with getDb() method

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

Change #1056068 merged by Zoranzoki21:

[mediawiki/extensions/BlueSpicePageTemplates@REL1_39-4.5.x] Tests:Replace "db" with getDb() method

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

Change #1052332 merged by jenkins-bot:

[mediawiki/extensions/EntitySchema@master] Test: Replace db with getDb method

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

Change #1053329 merged by Umherirrender:

[mediawiki/extensions/HoneyPot@master] Test:Replace db with getDb method

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

Change #1053287 merged by Umherirrender:

[mediawiki/extensions/FacetedCategory@master] Test:Replace db with getDb method

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

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

[mediawiki/extensions/ReadingLists@master] tests: Remove explicit clean up for database tests

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

Change #1053285 merged by jenkins-bot:

[mediawiki/extensions/MissedPages@master] Test:Replace db with getDb method

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

Change #1053286 merged by jenkins-bot:

[mediawiki/extensions/LDAPProvider@master] Test:Replace db with getDb method

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

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

[mediawiki/extensions/Flow@master] Test:Replace db with getDb method

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

Change #1061162 merged by jenkins-bot:

[mediawiki/extensions/Flow@master] Test:Replace db with getDb method

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

Change #1061160 merged by jenkins-bot:

[mediawiki/extensions/ReadingLists@master] tests: Do not create dummy project in setup of ReadingListRepositoryTest

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