[go: up one dir, main page]
Page MenuHomePhabricator
Create Task
Maniphest T312709

Parsoid/FlaggedRevs test failures
Open, HighPublic
Actions

Description

Seen on https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/812492

https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php72-noselenium-docker/162721/console

02:08:31 There was 1 error:
02:08:31 
02:08:31 1) ParserIntegrationTest::testParse with data set "citeParserTests.txt: Simple <ref>, no <references/>" ('legacy')
02:08:31 Error: Call to a member function getTemplateIds() on null in /workspace/src/extensions/FlaggedRevs/backend/FlaggedRevs.php:751
02:08:31 Stack trace:
02:08:31 #0 /workspace/src/extensions/FlaggedRevs/backend/FlaggedRevsHooks.php(507): FlaggedRevs::autoReviewEdit(Object(FlaggableWikiPage), Object(User), Object(MediaWiki\Revision\RevisionStoreRecord), Array)
02:08:31 #1 /workspace/src/includes/HookContainer/HookContainer.php(338): FlaggedRevsHooks::maybeMakeEditReviewed(Object(WikiPage), Object(MediaWiki\Revision\RevisionStoreRecord), false, Object(User), Array)
02:08:31 #2 /workspace/src/includes/HookContainer/HookContainer.php(137): MediaWiki\HookContainer\HookContainer->callLegacyHook('RevisionFromEdi...', Array, Array, Array)
02:08:31 #3 /workspace/src/includes/HookContainer/HookRunner.php(3209): MediaWiki\HookContainer\HookContainer->run('RevisionFromEdi...', Array)
02:08:31 #4 /workspace/src/includes/Storage/PageUpdater.php(1538): MediaWiki\HookContainer\HookRunner->onRevisionFromEditComplete(Object(WikiPage), Object(MediaWiki\Revision\RevisionStoreRecord), false, Object(User), Array)
02:08:31 #5 /workspace/src/includes/Storage/PageUpdater.php(942): MediaWiki\Storage\PageUpdater->doCreate(Object(CommentStoreComment))
02:08:31 #6 /workspace/src/includes/page/WikiPage.php(1905): MediaWiki\Storage\PageUpdater->saveRevision(Object(CommentStoreComment), 137)
02:08:31 #7 /workspace/src/tests/parser/ParserTestRunner.php(2541): WikiPage->doUserEditContent(Object(WikitextContent), Object(User), Object(CommentStoreComment), 137)
02:08:31 #8 /workspace/src/tests/parser/ParserTestRunner.php(2440): ParserTestRunner->addArticle('Template:nested...', '<ref><ref name=...', '/workspace/src/...', 2249)
02:08:31 #9 /workspace/src/tests/phpunit/suites/ParserTestFileSuite.php(59): ParserTestRunner->addArticles(Array)
02:08:31 #10 /workspace/src/tests/phpunit/suites/SuiteEventsTrait.php(34): ParserTestFileSuite->setUp()
02:08:31 #11 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestSuite.php(619): ParserTestFileSuite->run(Object(PHPUnit\Framework\TestResult))
02:08:31 #12 /workspace/src/tests/phpunit/suites/SuiteEventsTrait.php(69): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
02:08:31 #13 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestSuite.php(619): ParserTestTopLevelSuite->run(Object(PHPUnit\Framework\TestResult))
02:08:31 #14 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestSuite.php(619): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
02:08:31 #15 /workspace/src/vendor/phpunit/phpunit/src/TextUI/TestRunner.php(659): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
02:08:31 #16 /workspace/src/vendor/phpunit/phpunit/src/TextUI/Command.php(235): PHPUnit\TextUI\TestRunner->doRun(Object(PHPUnit\Framework\TestSuite), Array, Array, true)
02:08:31 #17 /workspace/src/tests/phpunit/phpunit.php(101): PHPUnit\TextUI\Command->run(Array, true)
02:08:31 #18 /workspace/src/tests/phpunit/phpunit.php(153): PHPUnitMaintClass->execute()
02:08:31 #19 {main}
02:08:31 --
02:08:31 
02:08:31 There were 6 failures:
02:08:31 
02:08:31 1) ParserIntegrationTest::testParse with data set "legacyMediaParserTests.txt: Bad images - basic functionality" ('legacy')
02:08:31 Failed asserting that two strings are equal.
02:08:31 --- Expected
02:08:31 +++ Actual
02:08:31 @@ @@
02:08:31 -'<p><a href="/wiki/File:Bad.jpg" title="File:Bad.jpg">File:Bad.jpg</a>\n
02:08:31 +'<p><a href="/wiki/File:Bad.jpg" class="image"><img alt="Bad.jpg" src="http://example.com/images/0/09/Bad.jpg" decoding="async" width="320" height="240" /></a>\n
02:08:31  </p>'
02:08:31 
02:08:31 /workspace/src/tests/phpunit/suites/ParserIntegrationTest.php:68
02:08:31 /workspace/src/tests/phpunit/suites/SuiteEventsTrait.php:69
02:08:31 /workspace/src/tests/phpunit/suites/SuiteEventsTrait.php:69
02:08:31 === Logs generated by test case
02:08:31 [wfDebug] [debug] ParserTestRunner::runLegacyTest: running Bad images - basic functionality {"private":false}
02:08:31 [wfDebug] [debug] ParserFactory: using default preprocessor {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/320px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] MockImageHandler::doFakeTransform: returning unscaled image {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/320px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] MockImageHandler::doFakeTransform: returning unscaled image {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/320px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] MockImageHandler::doFakeTransform: returning unscaled image {"private":false}
02:08:31 ===
02:08:31 
02:08:31 2) ParserIntegrationTest::testParse with data set "legacyMediaParserTests.txt: Bad images - T18039: text after bad image disappears" ('legacy')
02:08:31 Failed asserting that two strings are equal.
02:08:31 --- Expected
02:08:31 +++ Actual
02:08:31 @@ @@
02:08:31  '<p>Foo bar\n
02:08:31 -<a href="/wiki/File:Bad.jpg" title="File:Bad.jpg">File:Bad.jpg</a>\n
02:08:31 +<a href="/wiki/File:Bad.jpg" class="image"><img alt="Bad.jpg" src="http://example.com/images/0/09/Bad.jpg" decoding="async" width="320" height="240" /></a>\n
02:08:31  Bar foo\n
02:08:31  </p>'
02:08:31 
02:08:31 /workspace/src/tests/phpunit/suites/ParserIntegrationTest.php:68
02:08:31 /workspace/src/tests/phpunit/suites/SuiteEventsTrait.php:69
02:08:31 /workspace/src/tests/phpunit/suites/SuiteEventsTrait.php:69
02:08:31 === Logs generated by test case
02:08:31 [wfDebug] [debug] ParserTestRunner::runLegacyTest: running Bad images - T18039: text after bad image disappears {"private":false}
02:08:31 [wfDebug] [debug] ParserFactory: using default preprocessor {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/320px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] MockImageHandler::doFakeTransform: returning unscaled image {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/320px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] MockImageHandler::doFakeTransform: returning unscaled image {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/320px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] MockImageHandler::doFakeTransform: returning unscaled image {"private":false}
02:08:31 ===
02:08:31 
02:08:31 3) ParserIntegrationTest::testParse with data set "legacyMediaParserTests.txt: Bad images - in gallery" ('legacy')
02:08:31 Failed asserting that two strings are equal.
02:08:31 --- Expected
02:08:31 +++ Actual
02:08:31 @@ @@
02:08:31  '<ul class="gallery mw-gallery-traditional">\n
02:08:31  		<li class="gallerybox" style="width: 155px"><div style="width: 155px">\n
02:08:31 -			<div class="thumb" style="height: 150px;"><a href="/wiki/File:Bad.jpg" title="File:Bad.jpg">Bad.jpg</a></div>\n
02:08:31 +			<div class="thumb" style="width: 150px;"><div style="margin:30px auto;"><a href="/wiki/File:Bad.jpg" class="image"><img alt="Bad.jpg" src="http://example.com/images/thumb/0/09/Bad.jpg/120px-Bad.jpg" decoding="async" width="120" height="90" srcset="http://example.com/images/thumb/0/09/Bad.jpg/180px-Bad.jpg 1.5x, http://example.com/images/thumb/0/09/Bad.jpg/240px-Bad.jpg 2x" /></a></div></div>\n
02:08:31  			<div class="gallerytext">\n
02:08:31  			</div>\n
02:08:31  		</div></li>\n
02:08:31  </ul>'
02:08:31 
02:08:31 /workspace/src/tests/phpunit/suites/ParserIntegrationTest.php:68
02:08:31 /workspace/src/tests/phpunit/suites/SuiteEventsTrait.php:69
02:08:31 /workspace/src/tests/phpunit/suites/SuiteEventsTrait.php:69
02:08:31 === Logs generated by test case
02:08:31 [wfDebug] [debug] ParserTestRunner::runLegacyTest: running Bad images - in gallery {"private":false}
02:08:31 [wfDebug] [debug] ParserFactory: using default preprocessor {"private":false}
02:08:31 [wfDebug] [debug] ImageGalleryBase::factory called without context. Using RequestContext::getMain() {"private":false}
02:08:31 [wfDebug] [debug] ImageGallery::add Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/120px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/180px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/240px-Bad.jpg {"private":false}
02:08:31 ===
02:08:31 
02:08:31 4) ParserIntegrationTest::testParse with data set "mediaParserTests.txt: Bad images - basic functionality" ('legacy')
02:08:31 Failed asserting that two strings are equal.
02:08:31 --- Expected
02:08:31 +++ Actual
02:08:31 @@ @@
02:08:31 -'<p><a href="/wiki/File:Bad.jpg" title="File:Bad.jpg">File:Bad.jpg</a>\n
02:08:31 +'<p><span class="mw-default-size" typeof="mw:File"><a href="/wiki/File:Bad.jpg" class="mw-file-description"><img alt="Bad.jpg" src="http://example.com/images/0/09/Bad.jpg" decoding="async" width="320" height="240" /></a></span>\n
02:08:31  </p>'
02:08:31 
02:08:31 /workspace/src/tests/phpunit/suites/ParserIntegrationTest.php:68
02:08:31 /workspace/src/tests/phpunit/suites/SuiteEventsTrait.php:69
02:08:31 /workspace/src/tests/phpunit/suites/SuiteEventsTrait.php:69
02:08:31 === Logs generated by test case
02:08:31 [wfDebug] [debug] ParserTestRunner::runLegacyTest: running # FIXME: Legacy output doesn't match ParsoidBad images - basic functionality {"private":false}
02:08:31 [wfDebug] [debug] ParserFactory: using default preprocessor {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/320px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] MockImageHandler::doFakeTransform: returning unscaled image {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/320px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] MockImageHandler::doFakeTransform: returning unscaled image {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/320px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] MockImageHandler::doFakeTransform: returning unscaled image {"private":false}
02:08:31 ===
02:08:31 
02:08:31 5) ParserIntegrationTest::testParse with data set "mediaParserTests.txt: Bad images - T18039: text after bad image disappears" ('legacy')
02:08:31 Failed asserting that two strings are equal.
02:08:31 --- Expected
02:08:31 +++ Actual
02:08:31 @@ @@
02:08:31  '<p>Foo bar\n
02:08:31 -<a href="/wiki/File:Bad.jpg" title="File:Bad.jpg">File:Bad.jpg</a>\n
02:08:31 +<span class="mw-default-size" typeof="mw:File"><a href="/wiki/File:Bad.jpg" class="mw-file-description"><img alt="Bad.jpg" src="http://example.com/images/0/09/Bad.jpg" decoding="async" width="320" height="240" /></a></span>\n
02:08:31  Bar foo\n
02:08:31  </p>'
02:08:31 
02:08:31 /workspace/src/tests/phpunit/suites/ParserIntegrationTest.php:68
02:08:31 /workspace/src/tests/phpunit/suites/SuiteEventsTrait.php:69
02:08:31 /workspace/src/tests/phpunit/suites/SuiteEventsTrait.php:69
02:08:31 === Logs generated by test case
02:08:31 [wfDebug] [debug] ParserTestRunner::runLegacyTest: running # FIXME: Legacy output doesn't match ParsoidBad images - T18039: text after bad image disappears {"private":false}
02:08:31 [wfDebug] [debug] ParserFactory: using default preprocessor {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/320px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] MockImageHandler::doFakeTransform: returning unscaled image {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/320px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] MockImageHandler::doFakeTransform: returning unscaled image {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/320px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] MockImageHandler::doFakeTransform: returning unscaled image {"private":false}
02:08:31 ===
02:08:31 
02:08:31 6) ParserIntegrationTest::testParse with data set "mediaParserTests.txt: Bad images - in gallery" ('legacy')
02:08:31 Failed asserting that two strings are equal.
02:08:31 --- Expected
02:08:31 +++ Actual
02:08:31 @@ @@
02:08:31  '<ul class="gallery mw-gallery-traditional">\n
02:08:31  		<li class="gallerybox" style="width: 155px">\n
02:08:31 -			<div class="thumb" style="height: 150px;"><span typeof="mw:Error mw:File"><a href="/index.php?title=Special:Upload&amp;wpDestFile=Bad.jpg" class="new" title="File:Bad.jpg"><span class="mw-broken-media" data-width="120" data-height="120">File:Bad.jpg</span></a></span></div>\n
02:08:31 +			<div class="thumb" style="width: 150px; height: 150px;"><span typeof="mw:File"><a href="/wiki/File:Bad.jpg" class="mw-file-description"><img alt="Bad.jpg" src="http://example.com/images/thumb/0/09/Bad.jpg/120px-Bad.jpg" decoding="async" width="120" height="90" srcset="http://example.com/images/thumb/0/09/Bad.jpg/180px-Bad.jpg 1.5x, http://example.com/images/thumb/0/09/Bad.jpg/240px-Bad.jpg 2x" /></a></span></div>\n
02:08:31  			<div class="gallerytext">\n
02:08:31  			</div>\n
02:08:31  		</li>\n
02:08:31  </ul>'
02:08:31 
02:08:31 /workspace/src/tests/phpunit/suites/ParserIntegrationTest.php:68
02:08:31 /workspace/src/tests/phpunit/suites/SuiteEventsTrait.php:69
02:08:31 /workspace/src/tests/phpunit/suites/SuiteEventsTrait.php:69
02:08:31 === Logs generated by test case
02:08:31 [wfDebug] [debug] ParserTestRunner::runLegacyTest: running Bad images - in gallery {"private":false}
02:08:31 [wfDebug] [debug] ParserFactory: using default preprocessor {"private":false}
02:08:31 [wfDebug] [debug] ImageGalleryBase::factory called without context. Using RequestContext::getMain() {"private":false}
02:08:31 [wfDebug] [debug] ImageGallery::add Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/120px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/180px-Bad.jpg {"private":false}
02:08:31 [wfDebug] [debug] File::transform: Doing stat for mwstore://local-backend/local-thumb/0/09/Bad.jpg/240px-Bad.jpg {"private":false}

Details

SubjectRepoBranchLines +/-
Add missing null check to FlaggedRevs::autoReviewEdit()mediawiki/extensions/FlaggedRevsmaster+4 -0
Make current PreparedUpdate available to autoreviewEditmediawiki/extensions/FlaggedRevsmaster+41 -19
test: installMockMwServices should apply overrides per default.mediawiki/coremaster+42 -19
Switch back to CACHE_NONE temporarilymediawiki/coremaster+2 -2
Temporary patch to dump debug info if status is notOk in CImediawiki/extensions/FlaggedRevsmaster+7 -2
Customize query in gerrit

Related Objects

Event Timeline

Reedy created this task.Jul 10 2022, 1:15 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 10 2022, 1:15 AM
Reedy triaged this task as High priority.Jul 10 2022, 1:15 AM
Lens0021 subscribed.Jul 10 2022, 4:53 PM
Arlolra subscribed.Jul 14 2022, 3:33 PM

The non-cite tests seem related to,
https://github.com/wikimedia/mediawiki/commit/8d48b61ed3d66957c31dc75ce171b74467adccfe
https://github.com/wikimedia/mediawiki/commit/baf72a9b6c84829268e4d20f8c1f121147dcbeec

It looks like the BadFileLookup service got wedged and isn't returning the right results, and the output changes whether the file is bad or not.

kostajh subscribed.Jul 14 2022, 9:03 PM
Reedy raised the priority of this task from High to Unbreak Now!.Jul 14 2022, 9:20 PM

All merges seem to be currently blocked :(

ssastry subscribed.Jul 14 2022, 10:07 PM

So, if there are any exceptions while setting up articles, the service reset for BadFileLookup will not run.

Looking at the FlaggedRevFailure stack trace, it is crashing while setting up articles 02:08:31 #9 /workspace/src/tests/phpunit/suites/ParserTestFileSuite.php(59): ParserTestRunner->addArticles(Array) ... But, this should only have impacted the test run on citeParserTests.txt (which presumably would have aborted). So far, I haven't made sense of why the FlaggedRev extension is crashing now.

As for the other test failures, those shouldn't have been impacted by the FlaggedRev crasher since those failures are on a different test file. Those also seem to be more transient (and rechecks sometime pass these tests).

RhinosF1 subscribed.Jul 14 2022, 10:21 PM
Arlolra added a comment.Jul 14 2022, 11:37 PM

As for the other test failures, those shouldn't have been impacted by the FlaggedRev crasher since those failures are on a different test file. Those also seem to be more transient (and rechecks sometime pass these tests).

The way that the bad image list is populated is through article creation,

!! article
MediaWiki:bad image list
!! text
* [[File:Bad.jpg]] except [[Nasty page]]
!! endarticle
ssastry lowered the priority of this task from Unbreak Now! to High.Jul 15 2022, 3:02 AM

Arlo was investigating this and I was assisting him and but no luck with it since the recheck merged the patch this time. I am now retrying Daniel's patch. It is still a mystery. But, Arlo has a working theory and was hoping to dump an error message on failure ( https://gerrit.wikimedia.org/r/c/mediawiki/extensions/FlaggedRevs/+/814004 ) ... but it requires a failing patch now.

gerritbot added a comment.Jul 15 2022, 3:09 AM

Change 814004 had a related patch set uploaded (by Subramanya Sastry; author: Arlolra):

[mediawiki/extensions/FlaggedRevs@master] Temporary patch to dump debug info if status is notOk in CI

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

gerritbot added a project: Patch-For-Review.Jul 15 2022, 3:09 AM
gerritbot added a comment.Jul 15 2022, 3:12 AM

Change 814004 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Temporary patch to dump debug info if status is notOk in CI

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

Maintenance_bot removed a project: Patch-For-Review.Jul 15 2022, 3:30 AM
ReleaseTaggerBot added a project: MW-1.39-notes (1.39.0-wmf.21; 2022-07-18).Jul 15 2022, 4:00 AM
ssastry added a comment.EditedJul 15 2022, 1:56 PM

Okay, we finally got more info based on a CI failure on https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/814168.

The message from FlaggedRevs (in the jenkins job output linked above) is: Invariant failed: a:7:{s:9:"interface";b:1;s:8:"language";b:0;s:3:"key";s:16:"missing-revision";s:9:"keysToTry";a:1:{i:0;s:16:"missing-revision";}s:10:"parameters";a:1:{i:0;i:12;}s:11:"useDatabase";b:1;s:10:"titlevalue";N;}.

After running unserialize on the string, you get:

[
     "interface" => true,
     "language" => false,
     "key" => "missing-revision",
     "keysToTry" => [
       "missing-revision",
     ],
     "parameters" => [
       12,
     ],
     "useDatabase" => true,
     "titlevalue" => null,
]
ssastry added a comment.EditedJul 15 2022, 2:03 PM

Interestingly enough, there was another FlaggedRevs flappy test which was affecting Kartographer. See T312517 and in that case, the solution was to temporarily disable the test and later on Thiemo submitted a patch to detect failures and recover from it (not fixing it, but by marking the test incomplete).

I now think these two are related and probably the same source in FlaggedRev. Either something changed in FlaggedRevs or something in core broke some FlaggedRevs expectations.

Do we know who works on this extension that can help debug?

ssastry added a comment.Jul 15 2022, 6:37 PM

So, this is what @Arlolra and I have figured out:

  • This error seems to show up more consistently when zuul is loaded. So, all the CI failures yday afternoon CST. When I merged patches y'day night CST, there were no jobs in zuul, and i got to merge 2 patches. This morning, when zuul was loaded, even regular CI jobs (not gate-and-submit) failed on a Parsoid patch.
  • Different CI jobs are impacted (sometimes it is php72, sometimes php73, sometimes php74, and sometimes multiple
  • Sometimes the failures are on citeParserTests.txt, other times it is on legacyMediaParserTests.txt. But, it is always when adding articles from the test file that FlaggedRev hook handler crashes.

Looking into the code itself, here is what we find:

  • The exception message from FlaggedRevs pasted in T312709#8080850 indicates a 'missing-revision' error. That comes from line 293 in includes/page/ParserOutputAccess.php ... so it is failing revision lookup. Since it is article insertion, there is no parent revision id. But, WikiPage should have already inserted the page into the DB and associated the page id and the revision in WikiPage::updateRevisionOn(..).

So far, it looks some race condition between the database being update and then being looked up soon after in the hook runner.

The other mystery is why is this race condition triggering only in Parsoid's parser test runs vs. say parser tests runs for patches to core parser. But, maybe they will fail there too, it is just that Parser.php hasn't seen any code changes lately?

Any leads welcome, We'll keep dumping notes here as we understand better.

ssastry added a subscriber: thiemowmde.EditedJul 15 2022, 7:38 PM

@Arlolra and I think that perhaps https://gerrit.wikimedia.org/r/c/mediawiki/core/+/807926 and https://gerrit.wikimedia.org/r/c/mediawiki/core/+/807121 could have triggered this. @kostajh filed T312517 the same day which @gtisza and @thiemowmde worked around in Kartographer. Given the racy nature of this bug, and given that both those patches are touching caching in PHPUnit, we should look more into what changes those patches are introducing.

ssastry mentioned this in T312517: Build error: Kartographer\Tests\ApiQueryMapDataTest::testStableAndLatest.Jul 15 2022, 8:22 PM
gerritbot added a comment.Jul 15 2022, 9:24 PM

Change 814259 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/core@master] Switch back to CACHE_NONE temporarily

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

gerritbot added a project: Patch-For-Review.Jul 15 2022, 9:24 PM
Daimona subscribed.Jul 15 2022, 9:54 PM
daniel subscribed.Jul 16 2022, 9:26 AM

First insight:

ParserTestRunner calls MediaWikiIntegrationTestCase::installMockMwServices(). This used to call MediaWikiIntegrationTestCase::makeTestConfig() and pass the result as the bootstrap config to the new service container.

Per https://gerrit.wikimedia.org/r/c/mediawiki/core/+/808222, this is no longer the case:
makeTestConfig() has been replaced by getConfigOverrides(), and installMockMwServices() no longer calls it. Instead, overrideMwServices() calls it, and passes the Config to installMockMwServices().

For test cases that extend MediaWikiIntegrationtestCase, this doesn't make any difference. However, ParserTestRunner isn't - it just uses installMockMwServices() in isolation. So it's no longer getting any of the overrides.

I suspect this causes the it to use a real cache in CI, once that is shared between processes. Since these process will be configured to use the sdame cache keys, they will interfere with eachother.

Lens0021 unsubscribed.Jul 16 2022, 9:42 AM
gerritbot added a comment.Jul 16 2022, 10:02 AM

Change 814281 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] PHPUnit: Pass overrides to installMockMwServices.

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

gerritbot added a comment.Jul 16 2022, 3:12 PM

Change 814259 abandoned by Arlolra:

[mediawiki/core@master] Switch back to CACHE_NONE temporarily

Reason:

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

gerritbot added a comment.Jul 18 2022, 7:52 AM

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

[mediawiki/extensions/FlaggedRevs@master] Add missing null check to FlaggedRevs::autoReviewEdit()

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

gerritbot added a comment.Jul 18 2022, 7:12 PM

Change 814887 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/extensions/FlaggedRevs@master] Make current PreparedUpdate available to autoreviewEdit

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

gerritbot added a comment.Jul 18 2022, 8:38 PM

Change 814281 merged by jenkins-bot:

[mediawiki/core@master] test: installMockMwServices should apply overrides per default.

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

gerritbot added a comment.Jul 18 2022, 9:50 PM

Change 814710 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Add missing null check to FlaggedRevs::autoReviewEdit()

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

gerritbot added a comment.Jul 19 2022, 3:18 PM

Change 814887 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Make current PreparedUpdate available to autoreviewEdit

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

Maintenance_bot removed a project: Patch-For-Review.Jul 19 2022, 3:30 PM
ReleaseTaggerBot edited projects, added MW-1.39-notes (1.39.0-wmf.22; 2022-07-25); removed MW-1.39-notes (1.39.0-wmf.21; 2022-07-18).Jul 19 2022, 4:00 PM
ssastry edited projects, added Parsoid (Tracking); removed Parsoid.Jul 23 2022, 1:39 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits