Page MenuHomePhabricator

MediaWiki PHPUnit tests fail when virtual domains are configured
Open, Needs TriagePublic

Description

When virtual domains are properly configured (local multi-DB setup), GrowthExperiments tests fail:

Wikimedia\Rdbms\DBQueryError : Error 1146: Table 'awiki.unittest_growthexperiments_mentor_mentee' doesn't exist
Function: GrowthExperiments\Mentorship\Store\DatabaseMentorStore::loadMentorUserUncached
Query: SELECT  gemm_mentor_id  FROM `unittest_growthexperiments_mentor_mentee` `growthexperiments_mentor_mentee`    WHERE gemm_mentee_id = 1 AND gemm_mentor_role = 'primary'  LIMIT 1  

 /var/www/html/w/includes/libs/Rdbms/Database/Database.php:1231
 /var/www/html/w/includes/libs/Rdbms/Database/Database.php:1215
 /var/www/html/w/includes/libs/Rdbms/Database/Database.php:1189
 /var/www/html/w/includes/libs/Rdbms/Database/Database.php:644
 /var/www/html/w/includes/libs/Rdbms/Database/Database.php:1369
 /var/www/html/w/includes/libs/Rdbms/Database/Database.php:1318
 /var/www/html/w/includes/libs/Rdbms/Database/DBConnRef.php:129
 /var/www/html/w/includes/libs/Rdbms/Database/DBConnRef.php:366
 /var/www/html/w/includes/libs/Rdbms/QueryBuilder/SelectQueryBuilder.php:779
 /var/www/html/w/extensions/GrowthExperiments/includes/Mentorship/Store/DatabaseMentorStore.php:47
 /var/www/html/w/extensions/GrowthExperiments/includes/Mentorship/Store/MentorStore.php:99
 /var/www/html/w/includes/libs/ObjectCache/WANObjectCache.php:1709
 /var/www/html/w/includes/libs/ObjectCache/WANObjectCache.php:1519
 /var/www/html/w/extensions/GrowthExperiments/includes/Mentorship/Store/MentorStore.php:95
 /var/www/html/w/extensions/GrowthExperiments/tests/phpunit/integration/Mentorship/Store/DatabaseMentorStoreTest.php:46

and similar. This happens because growthexperiments_mentor_mentee does not exist on the main DB, so cloning doesn't copy it into the test DB.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

@Urbanecm_WMF We already have the following in TestSetup.php, which should have addressed that in T384238.

mediawiki-core/.../TestSetup.php
		// Ensure code using virtual domains uses the local database for integration tests,
		// since most test code isn't aware of virtual domains (T384238).
		$wgVirtualDomainsMapping = [];

That change replaces one problem (tests overwriting local data) with another problem (tests failing). As far as I can see, that task only redirected the queries to the local DB, but nothing makes sure the local test DB has the tables _created_.

What happens here is this:

  1. Developer has two database servers: main and x1. To be sure newly created code accesses DB tables at the right servers, they only have eg. growthexperiments_mentor_mentee on x1, not on main. That way, if something tries to read growthexperiments_mentor_mentee on main (when actually using the wiki), the developer would see a fatal error (rather than a silent success). $wgVirtualDomainsMapping is used to point code at the right tables.
  2. When running tests, MediaWikiIntegrationTestCase::setupDatabaseWithTestPrefix is responsible for preparing the DB. CloneDatabase is used to copy table structure from the main DB (all tables on main DB are re-created: if table exists, unittest_table gets created).
  3. Main DB has no growthexperiments_mentor_mentee (see point 1), so unittest_growthexperiments_mentor_mentee does not get created.
  4. Nothing ever looks at what tables exist in virtual domains defined, and nothing creates them anywhere.
  5. Thanks to that same line (T384238), code is looking for unittest_growthexperiments_mentor_mentee in main DB when running tests.
  6. Such table is missing (point 3) => failure.

T384238 said the tests should "either use local tables or manage table prefixes and associated bookkeeping for each virtual domain". The first option was taken for the queries, but not for creating the tables.

What needs to happen is that MediaWikiIntegrationTestCase::setupDatabaseWithTestPrefix needs to also check virtual domains defined, and clone their tables into the main test DB (assuming we want to empty the virtual domains mapping when running tests). setupExternalStoreTestDBs() already does something similar for ExternalStore connections. I'm not 100% sure why simply copying all tables on all virtual domains into their unittest_ variants isn't an option, but I might be missing something about how this is all wired up.

Alternatively, developers can have (empty) growthexperiments_mentor_mentee on main (despite the table really belonging to x1), so the clonning process will create them on main. However, then detecting "my code accidentally accesses wrong DB" would be much harder, defeating the point to use virtual domains locally to begin with. In addition to that, there is not a standard way to achieve that (update.php only creates the tables on x1; a main copy would need to be created manually).

Hope this clarifies.

Urbanecm_WMF renamed this task from GrowthExperiments tests fail when virtual domains are configured to MediaWiki PHPUnit tests fail when virtual domains are configured.Wed, Aug 19, 2:50 PM

CentralAuth handles that by unsetting the virtual DB mapping + running the schema update manually from UnitTestsAfterDatabaseSetup. T366433: CentralAuth tests broken unless you run them inside Quibble is a somewhat related task with some code pointers.

MediaWiki taking care of it would certainly be nicer, no idea how that would look architecturally though. Maybe the entire LBFactory - LB - DB hierarchy should have a test mode flag. Messing it up somehow and accidentally deleting production data is a scary prospect though.

That change replaces one problem (tests overwriting local data) with another problem (tests failing). […]

I don't think it fails per-se, but it fails in the specific case where you:

  • proactively configure a virtual domain mapping in LocalSettings.php before installing the relevant extension,
  • only run maintenance/update.php when those settings are active (i.e. never use or test the extension in single wiki or single db mode)

For me that's not the case as I only enable wikifarm on my local for specific tasks, and still run the updater plenty other days to have the tables exist and be updated on the main database.

But yeah, given that CloneDatabase already creates an empty version of the current database, maybe we can go a step further and not even bother with a clone but rather create them from scratch using the schemas. That should solve this issue. I don't thikn it'd be that much slower?