Flaky tests ParserOutputAccessTest::testOldRevisionCacheSplit and ::testOldRevisionUseCached
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Umherirrender
	Jan 4 2021, 12:58 AM

Description

There is a flaky test in core, which makes the merge of https://gerrit.wikimedia.org/r/c/mediawiki/core/+/653551 a bit complicated

It fails on postgres, but also on php7.3

There was 1 failure:

1) ParserOutputAccessTest::testOldRevisionCacheSplit
French output should be in the cache
Failed asserting that null is not null.

/workspace/src/tests/phpunit/includes/page/ParserOutputAccessTest.php:532
/workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:437
/workspace/src/maintenance/doMaintenance.php:106

and also

ParserOutputAccessTest::testOldRevisionUseCached
Expectation failed for method name is "getRenderedRevision" when invoked at most 1 times.
Expected invocation at most 1 times but it occurred 2 time(s).

/workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:446
/workspace/src/maintenance/doMaintenance.php:106

Details

	Subject	Repo	Branch	Lines +/-
	Mark broken ParserOutputAccess tests as broken	mediawiki/core	master	+4 -0
	Set proper epoch in cache tests.	mediawiki/core	master	+2 -2

Customize query in gerrit

Related Objects

Duplicates Merged Here: T271586: ParserOutputAccessTest::testOldRevisionCacheSplit seems to be flaky

Event Timeline

Umherirrender created this task.Jan 4 2021, 12:58 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 4 2021, 12:58 AM

DannyS712 merged a task: T271586: ParserOutputAccessTest::testOldRevisionCacheSplit seems to be flaky.Jan 8 2021, 8:56 PM

DannyS712 added a subscriber: Reedy.

Reedy added a project: Platform Engineering.Jan 8 2021, 10:18 PM

• holger.knust triaged this task as Medium priority.Jan 12 2021, 2:25 PM

• holger.knust edited projects, added Platform Team Workboards (Clinic Duty Team); removed Platform Engineering.

• holger.knust moved this task from Inbox to Later on the Platform Team Workboards (Clinic Duty Team) board.

• holger.knust added subscribers: • Pchelolo, daniel.

Another test in the same file is also failing on postgres

06:22:18 There was 1 failure:
06:22:18 
06:22:18 1) ParserOutputAccessTest::testOldRevisionUseCached
06:22:18 Expectation failed for method name is "getRenderedRevision" when invoked at most 1 times.
06:22:18 Expected invocation at most 1 times but it occurred 2 time(s).

Gonna boldly assign it to Daniel as the original author of that test suite

DannyS712 renamed this task from Flaky test ParserOutputAccessTest::testOldRevisionCacheSplit to Flaky tests ParserOutputAccessTest::testOldRevisionCacheSplit and ::testOldRevisionUseCached.Jan 16 2021, 11:37 PM

DannyS712 updated the task description. (Show Details)

Again, happenning repeatedly

20:02:37 There was 1 failure:
20:02:37 
20:02:37 1) ParserOutputAccessTest::testOldRevisionCacheSplit
20:02:37 French output should be in the cache
20:02:37 Failed asserting that null is not null.

Ladsgroup awarded a token.Jan 20 2021, 9:36 PM

Ladsgroup subscribed.

This is still happening randomly, failing unrelated patches during merge.
https://integration.wikimedia.org/ci/job/mediawiki-quibble-vendor-mysql-php74-docker/4329/console

Change 660912 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/core@master] Mark broken ParserOutputAccess tests as broken

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

gerritbot added a project: Patch-For-Review.Feb 1 2021, 7:37 PM

I don't know if it's fine to disable it. It seems like a rather important test.

The fact that's it's flaky at this rate shows there's logic problem with the test or the code it's testing. Since there's no fix upto now I don't see any harm in halting it, it can simply be reenabled anytime the issue is fixed. But leaving it hoping the issue to simply go away is wasting people's time.
There are many tests that are marked broken in core, I don't think all of them are marked because they're not important. It's simply because they're broken and no one has proposed a fix for them just like this one.

The fact that's it's flaky at this rate shows there's logic problem with the test or the code it's testing. Since there's no fix upto now I don't see any harm in halting it, it can simply be reenabled anytime the issue is fixed. But leaving it hoping the issue to simply go away is wasting people's time.

If the test is disabled, there is no incentive to fix it.

Personally, my problem with investigating is that it's random, and only on PostGres. That makes it hard to investigate, I don't know where to start.

There are many tests that are marked broken in core, I don't think all of them are marked because they're not important. It's simply because they're broken and no one has proposed a fix for them just like this one.

Nobody ever looks at them again. That's why there are many.

It not fail on postgres only, the task description also mention php7.3 which is running with mysql (that refers to job mediawiki-quibble-vendor-mysql-php73-docker). From my understanding there is no postgres explicit with php7.3

Hm, the error message indicates that something is dropped from the fake parser cache used by the tests. Which shouldn't be possible, since it's backed by a HashBagOStuff. Unless a very low expiry time is set, and the test is running very slowly. But the expiry time should be one hour, I can't imagine it running that slowly...

daniel edited projects, added Platform Team Workboards (MW Expedition); removed Platform Team Workboards (Clinic Duty Team).Feb 2 2021, 8:24 PM

daniel moved this task from Unsorted pile to Doing on the Platform Team Workboards (MW Expedition) board.Feb 3 2021, 5:43 PM

Ok, I was able to reproduce this by inserting sleep(1) between the two calls to getParserOutput() in testOldRevisionUseCached(). Seems like something is a bit too eager to expire...

Change 661513 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] Set proper epoch in cache tests.

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

daniel moved this task from Doing to Waiting for Review on the Platform Team Workboards (MW Expedition) board.Feb 3 2021, 10:15 PM

Change 661513 merged by jenkins-bot:
[mediawiki/core@master] Set proper epoch in cache tests.

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

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.30; 2021-02-09).Feb 3 2021, 11:00 PM

Change 660912 abandoned by Ammarpad:
[mediawiki/core@master] Mark broken ParserOutputAccess tests as broken

Reason: