Page MenuHomePhabricator

PageTriage: cleanup dangling test database danglers and remove defensive code in test cases
Closed, ResolvedPublic8 Estimated Story Points

Description

We've had to add deletion steps to several of our test cases to defend against entries left over from other test cases. In Idd7f6e8a0d1f460a19cc3e072ecd6b1c35bd2fb4, a question was raised about the code that cleaned up dangling entries on setup.

I did some investigation here and it looks like there is another test case is leaving pages behind. Here's an example amendment of the test where I've stopped manually clearing out pages.
https://gerrit.wikimedia.org/r/c/mediawiki/extensions/PageTriage/+/929784/1/tests/phpunit/MaintenanceCleanupPageTriagePageTagsTest.php

When I run the test by itself, it passes
php tests/phpunit/phpunit.php --filter 'MaintenanceCleanupPageTriagePageTagsTest'
when I run it along with all of the other tests it fails
php tests/phpunit/phpunit.php --filter PageTriage

To fix this, we should go through all the tests that are doing this defensive step and try to identify and eliminate the source of the polluted state and remove the defense against it

Event Timeline

jsn.sherman renamed this task from cleanup dangling test database danglers and remove defensive code in test cases to PageTriage: cleanup dangling test database danglers and remove defensive code in test cases.Jun 14 2023, 2:11 PM
Samwalton9-WMF set the point value for this task to 8.

When I run the test by itself, it passes
php tests/phpunit/phpunit.php --filter 'MaintenanceCleanupPageTriagePageTagsTest'
when I run it along with all of the other tests it fails
php tests/phpunit/phpunit.php --filter PageTriage

The reverse of this may also be true. I think we found an example of it in the Mod Tools NPP Office Hours call we had just now. For example when some of us run individual tests on SQLite, they fail, but when running the entire test suite, they pass.

We think this ticket is the root cause. Different databases (SQLite vs MariaDB) handle atomicity differently.

Samwalton9-WMF subscribed.

Moving to backlog for now, but this is still on our Q1 maintenance work list.

The following work for me on MariaDB:

docker compose exec mediawiki composer phpunit:entrypoint -- extensions/PageTriage/tests/phpunit/

docker compose exec mediawiki composer phpunit:entrypoint -- extensions/PageTriage/tests/phpunit/integration/MaintenanceCleanupPageTriageTest.php

Is this ticket only for SQLite?

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

[mediawiki/extensions/PageTriage@master] tests: No need to truncate the database

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

Change 990123 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] tests: No need to truncate the database

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