Page MenuHomePhabricator

Avoid creating a page and user for every PHPUnit Database test
Closed, ResolvedPublic

Description

MediaWikiIntegrationTestCase::addCoreDBData currently creates a user account and a page for every test (and for every Database test once r938440 will be merged). A lot of tests probably don't need this at all, thus resulting in a relatively large overhead.

We should stop doing that, and instead incourage the use of getTestSysop() / getTestUser() and getExistingTestPage(), which create the needed resources only if the test needs them.

Event Timeline

In theory, T155147 is not really needed for this task. But in practice, I'd rather get it done first so that the distinction of database vs non-database tests is real, so that we can really focus only on database tests for this task.

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

[mediawiki/core@master] [WIP] Make MediaWikiIntegrationTestCase::addCoreDBData a noop

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

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

[mediawiki/core@master] tests: Avoid relying on existence of a test page

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

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

[mediawiki/extensions/CheckUser@master] Do not rely on existence of UTSysop in HooksTest

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

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

[mediawiki/extensions/AntiSpoof@master] Do not rely on existence of test user in AntiSpoofPreAuthenticationProviderTest

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

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

[mediawiki/extensions/Echo@master] Do not assume what user IDs might be in DiscussionParserTest

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

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

[mediawiki/extensions/VisualEditor@master] Do not assume that UTPage exists in ApiVisualEditorTest

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

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

[mediawiki/extensions/Wikibase@master] Create full page in EntityIdLocalPartPageTableEntityQueryDbTest

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

Change 946557 merged by jenkins-bot:

[mediawiki/extensions/AntiSpoof@master] Do not rely on existence of test user in AntiSpoofPreAuthenticationProviderTest

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

Change 946554 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Do not rely on existence of UTSysop in HooksTest

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

Change 946560 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Do not assume that UTPage exists in ApiVisualEditorTest

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

Change 946558 merged by jenkins-bot:

[mediawiki/extensions/Echo@master] Do not assume what user IDs might be in DiscussionParserTest

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

Change 946552 merged by jenkins-bot:

[mediawiki/core@master] tests: Avoid relying on existence of a test page

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

Like in T155147#9072455, I ran non-database tests and here are the before and after the patch.

$ composer phpunit:entrypoint -- --group Database --exclude-group Broken,ParserFuzz,Stub,Standalone

Before:

Time: 07:14.917, Memory: 1.56 GB

ERRORS!
Tests: 13006, Assertions: 127001, Errors: 2, Skipped: 518, Incomplete: 4.

After:

Time: 05:34.650, Memory: 1.54 GB

ERRORS!
Tests: 13006, Assertions: 126995, Errors: 2, Skipped: 518, Incomplete: 4.

It is a single run, so maybe not particularly significant (I don't want to spend hours benchmarking this), but because the difference is so big, I think it is significant.

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

[mediawiki/core@master] Do not use UTSysop directly in auth tests

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

Change 949106 merged by jenkins-bot:

[mediawiki/core@master] Do not use UTSysop directly in auth tests

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

Comparing the CI runtime for the database suite:

https://integration.wikimedia.org/ci/job/wmf-quibble-core-vendor-mysql-php74-docker/25082/console has:

Time: 09:05.184, Memory: 2.33 GB

whereas for https://integration.wikimedia.org/ci/job/wmf-quibble-core-vendor-mysql-php74-docker/25083/console, that becomes:

Time: 07:27.281, Memory: 2.21 GB

These two jobs ran pretty much at the same time, 25082 is for r948191 (parent patch), and 25083 is for r946549, which makes addCoreDBData a noop.

Comparing the CI runtime for the database suite:

https://integration.wikimedia.org/ci/job/wmf-quibble-core-vendor-mysql-php74-docker/25082/console has:

Time: 09:05.184, Memory: 2.33 GB

whereas for https://integration.wikimedia.org/ci/job/wmf-quibble-core-vendor-mysql-php74-docker/25083/console, that becomes:

Time: 07:27.281, Memory: 2.21 GB

These two jobs ran pretty much at the same time, 25082 is for r948191 (parent patch), and 25083 is for r946549, which makes addCoreDBData a noop.

Very nice! 👏🏻

Change 946564 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] Create full page in EntityIdLocalPartPageTableEntityQueryDbTest

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

Change 946549 merged by jenkins-bot:

[mediawiki/core@master] Make MediaWikiIntegrationTestCase::addCoreDBData a noop

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

Change 955745 had a related patch set uploaded (by Jakob; author: Jakob):

[mediawiki/extensions/ArticlePlaceholder@master] Don't expect UTPage to exist in unit tests

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

Change 955745 merged by jenkins-bot:

[mediawiki/extensions/ArticlePlaceholder@master] Don't expect UTPage to exist in unit tests

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