FileImporter extension suite tests fail with UnexpectedValueException from FlaggedRevs
Closed, ResolvedPublic
Actions

Description

PHPUnit tests fail at the extension suite step with this:

18:37:31 1) ReviewHandlerTest::testWithAllParams
18:37:31 UnexpectedValueException: Flagged revision with ID 2 exists with unexpected fr_page_id
18:37:31 
18:37:31 /workspace/src/extensions/FlaggedRevs/business/RevisionReviewForm.php:503
18:37:31 /workspace/src/extensions/FlaggedRevs/business/RevisionReviewForm.php:308
18:37:31 /workspace/src/extensions/FlaggedRevs/business/FRGenericSubmitForm.php:192
18:37:31 /workspace/src/extensions/FlaggedRevs/frontend/specialpages/actions/RevisionReview.php:315
18:37:31 /workspace/src/extensions/FlaggedRevs/rest/ReviewHandler.php:25
18:37:31 /workspace/src/includes/Rest/SimpleHandler.php:38
18:37:31 /workspace/src/tests/phpunit/unit/includes/Rest/Handler/HandlerTestTrait.php:164
18:37:31 /workspace/src/extensions/FlaggedRevs/tests/phpunit/ReviewHandlerTest.php:72
18:37:31 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:518

See for example: https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php74-noselenium-docker/109339/consoleFull#console-section-17

CI was at least green until June 6th with tests executed in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/FileImporter/+/925839

I'm not sure if this is in any way related to this issue T256296: Fatal exception from FlaggedRevs: "Flagged revision with ID … exists with unexpected fr_page_id"

Details

Subject	Repo	Branch	Lines +/-
Add flaggedrevs table to $tablesUsed in ReviewHandlerTest	mediawiki/extensions/FlaggedRevs	REL1_40	+4 -3
Fix ReviewHandlerTest reusing existing test page name	mediawiki/extensions/FlaggedRevs	REL1_40	+12 -22
Add flaggedrevs table to $tablesUsed in ReviewHandlerTest	mediawiki/extensions/FlaggedRevs	REL1_39	+4 -3
Fix ReviewHandlerTest reusing existing test page name	mediawiki/extensions/FlaggedRevs	REL1_39	+12 -22
Drop all secondary caching of configuration parameters	mediawiki/extensions/FlaggedRevs	master	+18 -36
Make sure critical globals are set in ReviewHandlerTest	mediawiki/extensions/FlaggedRevs	master	+7 -7
Add flaggedrevs table to $tablesUsed in ReviewHandlerTest	mediawiki/extensions/FlaggedRevs	master	+4 -3
Fix ReviewHandlerTest reusing existing test page name	mediawiki/extensions/FlaggedRevs	master	+12 -24
Throw a more verbose error when data is corrupted	mediawiki/extensions/FlaggedRevs	master	+10 -7

Customize query in gerrit

Related Objects
Search...

Status	Assigned	Task
Open	None	T338439 Update MediaWiki API calls in TechWish products to use formatversion=2
Resolved	None	T254868 Use version 2 of APIs in FileImporter
Resolved	thiemowmde	T340004 FileImporter extension suite tests fail with UnexpectedValueException from FlaggedRevs

Event Timeline

WMDE-Fisch created this task.Jun 21 2023, 6:39 AM

WMDE-Fisch added projects: MediaWiki-extensions-FlaggedRevs, WMDE-TechWish-Maintenance-2023.Jun 21 2023, 6:44 AM

WMDE-Fisch updated the task description. (Show Details)

WMDE-Fisch added a subscriber: Umherirrender.

Umherirrender added a project: ci-test-error (WMF-deployed Build Failure).Jun 21 2023, 3:24 PM

WMDE-Fisch moved this task from Incoming to Priority backlog on the WMDE-TechWish-Maintenance board.Jun 23 2023, 10:47 AM

awight moved this task from Backlog to Doing on the WMDE-TechWish-Maintenance-2023 board.Jun 30 2023, 8:16 AM

thiemowmde claimed this task.Jun 30 2023, 8:50 AM

Current best guess is that a workaround for corruption similar to https://gerrit.wikimedia.org/r/c/mediawiki/extensions/FlaggedRevs/+/587849/ needs to be applied to the RevisionReview REST endpoint handler.

Change 934515 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/FlaggedRevs@master] Throw a more verbose error when data is corrupted

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

gerritbot added a project: Patch-For-Review.Jun 30 2023, 11:37 AM

Change 934583 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/FlaggedRevs@master] Drop all secondary caching of configuration parameters

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

Change 934659 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/FlaggedRevs@master] Fix ReviewHandlerTest reusing existing test page name

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

Getting somewhere. Turns out the failing ReviewHandlerTest in MediaWiki-extensions-FlaggedRevs is somewhat broken. It uses the same page name "UTPage" for everything, which not only causes conflicts with tests from other extensions that happen to use the same page, it also means the individual ReviewHandlerTest cases aren't independent from each other. Our "why is change-time null at the end?" confusion is because the edit that sets the page content to "SecondEdit" doesn't do anything on a page that already has that content.

https://gerrit.wikimedia.org/r/934659 fixes ReviewHandlerTest. Rerunning the FileImporter tests with this as a dependency finally gets rid of most of the fr_page_id failures.

Unfortunately ReviewHandlerTest::testWithAllParams keeps failing with "Someone already accepted or unaccepted this revision while you were viewing it", but only when run as part of the FileImporter tests. I think this was partly because I tried to compare a change-time that ended being off by one second on CI hardware. I couldn't reproduce this locally. I removed the strict comparison and went back to the original "there should be a change-time, but the value doesn't matter".

Change 934515 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Throw a more verbose error when data is corrupted

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

ReleaseTaggerBot added a project: MW-1.41-notes (1.41.0-wmf.16; 2023-07-04).Jul 3 2023, 4:00 PM

thiemowmde added a parent task: T254868: Use version 2 of APIs in FileImporter.Jul 5 2023, 7:47 AM

Change 934659 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Fix ReviewHandlerTest reusing existing test page name

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

ReleaseTaggerBot edited projects, added MW-1.41-notes (1.41.0-wmf.17; 2023-07-11); removed MW-1.41-notes (1.41.0-wmf.16; 2023-07-04).Jul 5 2023, 2:00 PM

Change 935764 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/FlaggedRevs@master] Make sure critical globals are set in ReviewHandlerTest

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

Lucas_Werkmeister_WMDE mentioned this in T341210: FlaggedRevs ReviewHandlerTest::testWithMinParams fails in EntitySchema CI.Jul 6 2023, 11:36 AM

Change 936264 had a related patch set uploaded (by Thiemo Kreuz (WMDE); author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/FlaggedRevs@master] [DNM] Empty flaggedrevs table in ReviewHandlerTest

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

Lucas_Werkmeister_WMDE mentioned this in T341344: Make MediaWikiIntegrationTestCase::getExistingTestPage() default to random title.Jul 7 2023, 1:22 PM

Change 936264 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Add flaggedrevs table to $tablesUsed in ReviewHandlerTest

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

thiemowmde closed this task as Resolved.Jul 7 2023, 1:48 PM

WMDE-Fisch moved this task from Doing to Done on the WMDE-TechWish-Maintenance-2023 board.Jul 7 2023, 7:48 PM

Change 935764 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Make sure critical globals are set in ReviewHandlerTest

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

ReleaseTaggerBot edited projects, added MW-1.41-notes (1.41.0-wmf.18; 2023-07-18); removed MW-1.41-notes (1.41.0-wmf.17; 2023-07-11).Jul 17 2023, 8:00 PM

Change 934583 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Drop all secondary caching of configuration parameters

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

ReleaseTaggerBot edited projects, added MW-1.41-notes (1.41.0-wmf.22; 2023-08-15); removed MW-1.41-notes (1.41.0-wmf.18; 2023-07-18).Aug 4 2023, 8:00 PM

Maintenance_bot removed a project: Patch-For-Review.Aug 4 2023, 8:10 PM

Change 963045 had a related patch set uploaded (by Umherirrender; author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/FlaggedRevs@REL1_40] Add flaggedrevs table to $tablesUsed in ReviewHandlerTest

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

gerritbot added a project: Patch-For-Review.Oct 3 2023, 8:13 PM

Change 963166 had a related patch set uploaded (by Umherirrender; author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/FlaggedRevs@REL1_40] Fix ReviewHandlerTest reusing existing test page name

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

Change 963167 had a related patch set uploaded (by Umherirrender; author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/FlaggedRevs@REL1_39] Fix ReviewHandlerTest reusing existing test page name

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

Change 963168 had a related patch set uploaded (by Umherirrender; author: Thiemo Kreuz (WMDE)):

[mediawiki/extensions/FlaggedRevs@REL1_39] Add flaggedrevs table to $tablesUsed in ReviewHandlerTest

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

Change 963167 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@REL1_39] Fix ReviewHandlerTest reusing existing test page name

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

Change 963168 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@REL1_39] Add flaggedrevs table to $tablesUsed in ReviewHandlerTest

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

Change 963166 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@REL1_40] Fix ReviewHandlerTest reusing existing test page name

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

Change 963045 merged by jenkins-bot: