Page MenuHomePhabricator

EntitySchema CI broken since IDatabase alias was removed
Closed, ResolvedPublic

Description

Since the \IDatabase aliases for \Wikimedia\Rdbms\IDatabase was removed (T344536), two tests in EntitySchema are failing (build):

1) EntitySchema\Tests\Integration\MediaWiki\Actions\RestoreSubmitActionTest::testRestoreSubmit
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'abc'
+'def'

/workspace/src/extensions/EntitySchema/tests/phpunit/integration/MediaWiki/Actions/RestoreSubmitActionTest.php:72

2) EntitySchema\Tests\Integration\MediaWiki\Actions\RestoreViewActionTest::testRestoreView
Failed asserting that '<div class="mw-message-box cdx-message cdx-message--block mw-message-box-error cdx-message--error"><span class="cdx-message__icon"></span><div class="cdx-message__content">The revision you tried to restore is identical to the current revision. Nothing to do.</div></div><p id="mw-returnto">Return to <a href="/index.php/EntitySchema:E1234" title="EntitySchema:E1234">EntitySchema:E1234</a>.</p>\n
' contains "<ins class="diffchange diffchange-inline">abc</ins>".

/workspace/src/extensions/EntitySchema/tests/phpunit/integration/MediaWiki/Actions/RestoreViewActionTest.php:82

We need to figure out how to fix these tests; if we can’t figure it out, we should temporarily restore the \IDatabase alias to fix CI.

Event Timeline

So far, what I’ve noticed with the debugger is that RestoreSubmitActionTest::testRestoreSubmit() creates two EntitySchema revisions, which are assigned revids 2 and 3; RestoreSubmitAction then looks up the latest revision and compares it to the given wpBaseRev to see if there was an edit conflict in the meantime. With the class alias present, it correctly gets 3 as the latest revision ID; with the alias removed, it instead gets 2. It feels like some kind of instance cache isn’t being cleared between test runs test edits anymore, but I have no clue how the class alias can have that effect.

And it seems to be this fun kind of bug that vanishes if you spend too much time stepping through it with the debugger (race condition?).

Change 953277 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/FlaggedRevs@master] Add missing import for namespaced IDatabase

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

Change 953278 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/EntitySchema@master] DNM: empty change to test CI

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

And it seems to be this fun kind of bug that vanishes if you spend too much time stepping through it with the debugger (race condition?).

I knew it felt familiar – Michael found T334464: EntitySchema PHPUnit tests fail when FlaggedRevs is installed, where I think the same thing happened (notice the TTL_MINUTE in T334464#8770898).

Change 953277 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] Add missing import for namespaced IDatabase

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

Change 953278 abandoned by Lucas Werkmeister (WMDE):

[mediawiki/extensions/EntitySchema@master] DNM: empty change to test CI

Reason:

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

Lucas_Werkmeister_WMDE claimed this task.

Now it’s working again. Thanks everyone!