Possibly related: https://gerrit.wikimedia.org/g/mediawiki/vendor/+/c905e08871c70b06f6b15808484675f53bbad65b/wikimedia/services/src/ContainerDisabledException.php

Been investigating at https://gerrit.wikimedia.org/r/c/mediawiki/extensions/PageTriage/+/997634 by putting a bunch of log statements in and I'm a bit stumped. Here's where I'm at:

It seems that the test environment gets torn down early, when the test hasn't even finished yet. Somehow this reliably always happens at the same place every time. The apparent sequence of events goes like this:

1. Test starts.
2. "Undelete me" is created, then deleted.
3. Undeletion for "Undelete me" is triggered (goto).
4. It finishes undeleting revisions and updating site stats (goto).
5. onArticleUndelete hooks are called (goto).
6. Flow's undelete hooks (goto) get called.
7. Flow successfully creates the "Flow talk page manager" user (goto).
8. Flow applies the "bot" permission to that user (goto).
9. The test environment is torn down. (should not happen)
   • The trace at this point says that it was called from PhpUnit, at a point where the test should be done executing (called here, test runs here).
10. MediaWikiIntegrationTestCase (ancestor of HooksTest) calls destroy() on the mock services (set here as global)
11. Mock services are destroyed, and the service container is marked as destroyed (goto).
12. Flow tries to apply the "flow-bot" permission to that user (goto).
13. (now seen in the trace in the task description) addUserToGroup from the now-destroyed service container's UserGroupManager is called, and it then calls its callbacks (goto).
14. A callback to invalidate the user rights cache/user-related caches is called from core ServiceWiring.php (goto).
15. Either way, the callback tries to get services from the ServiceContainer, which is now destroyed. ServiceContainer::getService is called, $this->destroyed evaluates to true, an exception is thrown.
16. Test fails.

What I'm wondering is why a teardown happens when the test hasn't finished yet. The test should still be well within the undeleteAsUser call that the test shouldn't have returned early (which would cause the teardown to happen). My knowledge on PhpUnit hits is barrier here, since I'm not aware of any other condition that would cause the teardown to happen early. There's probably some side effect shenanigans going on here, or a silenced exception that isn't printed out in the debug logs to easily find out. I tried checking Gerrit for changes that occurred around the time tests started failing, but came up empty handed, so I'm not exactly sure what to try next.

I think StructuredDiscussions (Flow) is a really good clue here. When you checked recent patches, did you check the Flow repo?

Checking the Jenkins logs, the gate CI jobs don't mention flow at all, and the PageTriage non-gate CI jobs do mention Flow. This is the opposite of what I would expect. Why would the narrow PageTriage non-gate CI load a random extension like Flow? It is not a PageTriage dependency. Hmm.

Perhaps the fix is to patch the PageTriage non-gate CI to not load Flow. Flow is notoriously buggy and unmaintained.

I checked Flow but it was mostly localization and npm updates, so I wasn't ready to blame it just yet. In my head, this loop being interrupted by the teardown was pretty odd, and I focused on that instead.

Given that Flow is now pending removal everywhere (T332022), taking Flow out of CI tests sounds reasonable. The English Wikipedia doesn't have Flow anyway, and Flow would probably be gone by the time PageTriage becomes wiki-agnostic enough to install on wikis that would have used Flow.

I did a codesearch and couldn't easily find where the PageTriage CI is loading Flow.

https://codesearch.wmcloud.org/devtools/?q=PageTriage&files=&excludeFiles=&repos=

Another thing to try might be installing Flow locally, then running PHPUnit locally and seeing if we can replicate the error. If so, step debugging might reveal something.

Can't seem to replicate this locally, or at least I know too little of the configuration to ensure that I'm replicating this properly. The page is undeleted with no issues. But the debug log it spat out corroborated what I wrote above.

Here's an excerpt of the debug log leading up to the early teardown:

[rdbms] MediaWiki\User\UserGroupManager::getUserGroupMemberships [0.222ms] localhost:/workspace/db/quibble-mysql-0y233umb/socket: SELECT  ug_user,ug_group,ug_expiry  FROM `unittest_user_groups`    WHERE ug_user = 3  
[objectcache] getWithSetCallback(global:rdbms-server-readonly:localhost%3A/workspace/db/quibble-mysql-0y233umb/socket): process cache hit
[rdbms] Wikimedia\Rdbms\LoadBalancer::reuseOrOpenConnectionForNewRef: reusing connection for 0/wikidb-unittest_
[rdbms] MediaWiki\User\UserGroupManager::getUserGroupMemberships [0.203ms] localhost:/workspace/db/quibble-mysql-0y233umb/socket: SELECT  ug_user,ug_group,ug_expiry  FROM `unittest_user_groups`    WHERE ug_user = 3  
[rdbms] Wikimedia\Rdbms\LoadBalancer::reuseOrOpenConnectionForNewRef: reusing connection for 0/wikidb-unittest_
[rdbms] startAtomic: entering level 1 (MediaWiki\User\UserGroupManager::addUserToGroup)
[rdbms] MediaWiki\User\UserGroupManager::addUserToGroup [0.174ms] localhost:/workspace/db/quibble-mysql-0y233umb/socket: INSERT IGNORE INTO `unittest_user_groups` (ug_user,ug_group,ug_expiry) VALUES (3,'bot',NULL)
[rdbms] endAtomic: leaving level 1 (MediaWiki\User\UserGroupManager::addUserToGroup)
[PageTriageHooksTest] #0 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestCase.php(1200): MediaWiki\Extension\PageTriage\Test\HooksTest->tearDown()
#1 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestResult.php(728): PHPUnit\Framework\TestCase->runBare()
#2 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestCase.php(904): PHPUnit\Framework\TestResult->run()
#3 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestSuite.php(675): PHPUnit\Framework\TestCase->run()
#4 /workspace/src/vendor/phpunit/phpunit/src/Framework/TestSuite.php(675): PHPUnit\Framework\TestSuite->run()
...

compared to how it looks like when it runs successfully:

[rdbms] MediaWiki\User\UserGroupManager::getUserGroupMemberships [3.161ms] 127.0.0.1: SELECT  ug_user,ug_group,ug_expiry  FROM `unittest_user_groups`    WHERE ug_user = 3  
[objectcache] getWithSetCallback(global:rdbms-server-readonly:127.0.0.1): process cache hit
[rdbms] Wikimedia\Rdbms\LoadBalancer::reuseOrOpenConnectionForNewRef: reusing connection for 0/wiki_mwdev-unittest_
[rdbms] MediaWiki\User\UserGroupManager::getUserGroupMemberships [0.375ms] 127.0.0.1: SELECT  ug_user,ug_group,ug_expiry  FROM `unittest_user_groups`    WHERE ug_user = 3  
[rdbms] Wikimedia\Rdbms\LoadBalancer::reuseOrOpenConnectionForNewRef: reusing connection for 0/wiki_mwdev-unittest_
[rdbms] startAtomic: entering level 1 (MediaWiki\User\UserGroupManager::addUserToGroup)
[rdbms] MediaWiki\User\UserGroupManager::addUserToGroup [0.3ms] 127.0.0.1: INSERT IGNORE INTO `unittest_user_groups` (ug_user,ug_group,ug_expiry) VALUES (3,'bot',NULL)
[rdbms] endAtomic: leaving level 1 (MediaWiki\User\UserGroupManager::addUserToGroup)
[rdbms] Wikimedia\Rdbms\LoadBalancer::reuseOrOpenConnectionForNewRef: reusing connection for 0/wiki_mwdev-unittest_
[objectcache] getWithSetCallback(global:rdbms-server-readonly:127.0.0.1): process cache hit
[rdbms] Wikimedia\Rdbms\LoadBalancer::reuseOrOpenConnectionForNewRef: reusing connection for 0/wiki_mwdev-unittest_
[rdbms] startAtomic: entering level 1 (MediaWiki\User\UserGroupManager::addUserToGroup)
[rdbms] MediaWiki\User\UserGroupManager::addUserToGroup [0.292ms] 127.0.0.1: INSERT IGNORE INTO `unittest_user_groups` (ug_user,ug_group,ug_expiry) VALUES (3,'flow-bot',NULL)
[rdbms] endAtomic: leaving level 1 (MediaWiki\User\UserGroupManager::addUserToGroup)
[rdbms] Wikimedia\Rdbms\LoadBalancer::reuseOrOpenConnectionForNewRef: reusing connection for 0/wiki_mwdev-unittest_
[rdbms] endAtomic: leaving level 0 (MediaWiki\Page\UndeletePage::undeleteRevisions)
[rdbms] MediaWiki\Page\UndeletePage::undeleteRevisions [0.221ms] 127.0.0.1: COMMIT
...

Is it normal for an extension's CI to load a bunch of non-dependencies? The PageTriage CI is loading all these dependencies. Also I wonder how these are picked and where the code that picks them is?

["mediawiki/core"
"mediawiki/extensions/AbuseFilter"
"mediawiki/extensions/AntiSpoof"
"mediawiki/extensions/BetaFeatures"
"mediawiki/extensions/CentralAuth"
"mediawiki/extensions/CheckUser"
"mediawiki/extensions/Cite"
"mediawiki/extensions/CodeEditor"
"mediawiki/extensions/ConfirmEdit"
"mediawiki/extensions/DiscussionTools"
"mediawiki/extensions/Echo"
"mediawiki/extensions/EventBus"
"mediawiki/extensions/EventLogging"
"mediawiki/extensions/EventStreamConfig"
"mediawiki/extensions/FlaggedRevs"
"mediawiki/extensions/Flow"
"mediawiki/extensions/GuidedTour"
"mediawiki/extensions/Linter"
"mediawiki/extensions/MobileApp"
"mediawiki/extensions/MobileFrontend"
"mediawiki/extensions/ORES"
"mediawiki/extensions/PageTriage"
"mediawiki/extensions/ParserFunctions"
"mediawiki/extensions/Renameuser"
"mediawiki/extensions/Scribunto"
"mediawiki/extensions/SecurePoll"
"mediawiki/extensions/SyntaxHighlight_GeSHi"
"mediawiki/extensions/TemplateData"
"mediawiki/extensions/Thanks"
"mediawiki/extensions/TorBlock"
"mediawiki/extensions/UserMerge"
"mediawiki/extensions/VisualEditor"
"mediawiki/extensions/WikiEditor"
"mediawiki/extensions/WikiLove"
"mediawiki/extensions/WikimediaEvents"
"mediawiki/skins/MinervaNeue"
"mediawiki/skins/Vector"
"mediawiki/vendor"]

Ah, I guess CI just recurses through this list of dependencies:

https://gerrit.wikimedia.org/g/integration/config/+/3403859bb11bff1abf7f7813975c7cb7321458db/zuul/parameter_functions.py#104

• 'PageTriage': ['WikiLove', 'ORES', 'Echo'],
  • 'Echo': ['CentralAuth', 'EventLogging', 'MobileFrontend'],
    • 'CentralAuth': ['AbuseFilter', 'AntiSpoof', 'SecurePoll'],
      • 'AbuseFilter': ['AntiSpoof', 'CentralAuth', 'CodeEditor', 'CheckUser', 'Echo', 'Renameuser', 'Scribunto', 'EventLogging', 'UserMerge'],
    • 'EventLogging': ['EventStreamConfig', 'EventBus'],
    • 'MobileFrontend': ['Echo', 'VisualEditor', 'MobileApp', 'skins/MinervaNeue'],
      • 'VisualEditor': ['Cite', 'TemplateData', 'FlaggedRevs', 'ConfirmEdit', 'DiscussionTools'],
        • 'Cite': ['ParserFunctions', 'VisualEditor', 'WikiEditor'],
        • 'FlaggedRevs': ['Scribunto'],
        • 'DiscussionTools': ['VisualEditor', 'Linter', 'Echo', 'Thanks'],
      • 'MobileApp': ['Echo', 'MobileFrontend', 'VisualEditor', 'AbuseFilter'],
      • 'skins/MinervaNeue': ['MobileFrontend'],

I don't see Flow in that recursion tree yet, although I didn't finish manually recursing it. But if at any point we load MassMessage, Thanks, or GrowthExperiments, Flow will also be loaded. And maybe Flow can also be loaded via the Phan dependency tree a bit farther down.

https://gerrit.wikimedia.org/g/integration/config/+/3403859bb11bff1abf7f7813975c7cb7321458db/zuul/parameter_functions.py#554

• 'PageTriage': ['WikiLove', 'ORES', 'Echo'],
  • 'WikiLove': ['Flow', 'LiquidThreads', 'UserMerge'],

Change 997932 had a related patch set uploaded (by Jsn.sherman; author: Jsn.sherman):

[mediawiki/extensions/PageTriage@master] phpunit: replace deprecated methods in HooksTest

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

I spent some time looking at this today. I was not able to reproduce this locally by running:

docker compose exec mediawiki composer phpunit:entrypoint -- --testsuite extensions --group Database --exclude-group Broken,ParserFuzz,Stub,Standalone

which I adapted from the ci run:
https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php74-noselenium-docker/152432/console

I have wikilove and flow installed. I've done quite a bit of fiddling with the service setup in the test and hook code in a WIP patch, and my approach hasn't had an impact.

I have to move on for today, but I can come back to this tomorrow. Assuming it's still open, I think I'll look more closely at flow and wikilove.

Change 997994 had a related patch set uploaded (by Novem Linguae; author: Novem Linguae):

[mediawiki/extensions/PageTriage@master] phpunit: skip broken test

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

After some more testing, I've discovered that both testOnPageDelete() and testOnPageUndelete() are affected. $this->markTestSkipped() does not fix the error.

Change 997994 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] phpunit: delete 2 broken tests

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

Just noting that I rebased my patch and re-added the tests while we try to understand what is happening here.
https://gerrit.wikimedia.org/r/997932

I've certainly not made headway. I pinged a colleague for help on this internally in slack, but haven't heard back. I'll follow-up with a larger group. If that isn't fruitful, I'll see if there is a more formal way to request help with integration test issues like this. I will reopen if I can get someone with the right expertise engaged with this.

In T356704#9516168, @Chlod wrote:

Given that Flow is now pending removal everywhere (T332022), taking Flow out of CI tests sounds reasonable.

Yeah I would just drop Flow from the gated extensions. I think we talked about that a while ago but can't find the task.

If you want to debug what's happening, I think your best bet is checking out Quibble so you can run the test in the exact same environment it runs in on the CI servers, but with a debugger enabled.

In T356704#9556128, @Tgr wrote:

In T356704#9516168, @Chlod wrote:

Given that Flow is now pending removal everywhere (T332022), taking Flow out of CI tests sounds reasonable.

Yeah I would just drop Flow from the gated extensions. I think we talked about that a while ago but can't find the task.

I did this a year ago: 2d8f8fe3c.

To "fix" this, you'd need to drop it from individual dependencies, where it's currently listed as to be tested with GrowthExperiments, MassMessage, and Thanks. I believe at least for some if not all of these you'd need to delete or disable the relevant tests in each repo first. The teams responsible for those extensions might not be very comfortable about deleting test code for under-reviewed areas of their responsibility ahead of the extension being disabled.

In T356704#9559312, @Jdforrester-WMF wrote:

In T356704#9556128, @Tgr wrote:

In T356704#9516168, @Chlod wrote:

Given that Flow is now pending removal everywhere (T332022), taking Flow out of CI tests sounds reasonable.

Yeah I would just drop Flow from the gated extensions. I think we talked about that a while ago but can't find the task.

I did this a year ago: 2d8f8fe3c.

To "fix" this, you'd need to drop it from individual dependencies, where it's currently listed as to be tested with GrowthExperiments, MassMessage, and Thanks. I believe at least for some if not all of these you'd need to delete or disable the relevant tests in each repo first. The teams responsible for those extensions might not be very comfortable about deleting test code for under-reviewed areas of their responsibility ahead of the extension being disabled.

Yeah, we're here because of our Thanks dependency.

If you want to debug what's happening, I think your best bet is checking out Quibble so you can run the test in the exact same environment it runs in on the CI servers, but with a debugger enabled.

I found the relevant docs
https://doc.wikimedia.org/quibble/
and will see if I can get quibble up and running.

In T356704#9556128, @Tgr wrote:

If you want to debug what's happening, I think your best bet is checking out Quibble so you can run the test in the exact same environment it runs in on the CI servers, but with a debugger enabled.

I was trying to get Quibble running locally earlier, but it kept getting stuck on init: Wikimedia\Composer\Merge\V2\MergePlugin->onInit in Install composer dev-requires for vendor.git, never hitting the ./composer.json has been updated that I see in the Jenkins console log. Thought it was another Windows Docker bind mount issue, so I tried on Ubuntu, but got the same thing. Not sure what I did wrong there; will try again in a day or so.

In T356704#9560321, @Chlod wrote:

In T356704#9556128, @Tgr wrote:

If you want to debug what's happening, I think your best bet is checking out Quibble so you can run the test in the exact same environment it runs in on the CI servers, but with a debugger enabled.

I was trying to get Quibble running locally earlier, but it kept getting stuck on init: Wikimedia\Composer\Merge\V2\MergePlugin->onInit in Install composer dev-requires for vendor.git, never hitting the ./composer.json has been updated that I see in the Jenkins console log. Thought it was another Windows Docker bind mount issue, so I tried on Ubuntu, but got the same thing. Not sure what I did wrong there; will try again in a day or so.

I had this running in the background for quite a while before I noticed it was stalled out. I hit enter without thinking and it turns out there was a hidden prompt that I could only see after the fact:

> init: Wikimedia\Composer\Merge\V2\MergePlugin->onInit
nikic/php-parser is currently present in the require key and you ran the command with the --dev flag, which will move it to the require-dev key.
psy/psysh is currently present in the require key and you ran the command with the --dev flag, which will move it to the require-dev key.
wikimedia/langconv is currently present in the require key and you ran the command with the --dev flag, which will move it to the require-dev key.
wikimedia/testing-access-wrapper is currently present in the require key and you ran the command with the --dev flag, which will move it to the require-dev key.

Do you want to move these requirements? [no]? Do you want to re-run the command without --dev? [yes]? ./composer.json has been updated
> init: Wikimedia\Composer\Merge\V2\MergePlugin_composer_tmp0->onInit

It seemed to be running fine until it hit the selenium tests:

DEBUG:quibble.chromium_flags:Flags: ['', '--autoplay-policy=no-user-gesture-required', '--disable-pushstate-throttle', '--no-sandbox', '--headless', '--disable-gpu', '--remote-debugging-port=9222']
WARNING:backend.ChromeWebDriver:[1708461133.719][SEVERE]: bind() failed: Cannot assign requested address (99)
Running "assert-mw-env" task

I'll have to troubleshoot that tomorrow

Flow holds a reference to an older UserGroupManager, maybe the TalkpageManager already created in an previous test.

The undelete hook calls BoardMover, which needs the TalkpageManager where the bot group is added. Adding a group also calls some cache callbacks, these callbacks using the same service container where the UserGroupManager is from and that could be already destroyed.

$this->clearHook( 'ArticleUndelete' )

should avoid that Flow is called. Maybe fixing T170330 also helps to avoid that services are hanging around.
The flow base test class calls Container::reset(); to bypass the problems, call that in a class_exists or "Flow enabled" condition.

Maybe lazy create the system user in TalkpageManager as PageTriage does not interact with Flow namespace as far I could see.

In T356704#9561585, @Tgr wrote:

In T356704#9559312, @Jdforrester-WMF wrote:

I did this a year ago: 2d8f8fe3c.

Ah, right, I forgot about that.

GrowthExperiments, MassMessage, and Thanks have code which can only be tested in the presence of Flow so I guess not easily fixable. Maybe there should be a way to depend non-recursively on an extension (ie. when Jenkins checks a Thanks patch, it would install non-recursive dependencies of Thanks, but when it tests a patch from an extension that Thanks depends on, it wouldn't, and the relevant Thanks tests would mark themselves as skipped when Flow is not present), but that seems like a bigger project.

We do this for phan (as generally Foo calling Bar that implements X doesn't need to load X for internal static analysis), but not for unit/integration tests (as Foo calling Bar that calls X definitely does need X's code).

Theoretically we could do some form of automatic back-off of each pair-wise combination at test run time and see if they pass without the dependency, but yes, probably too much work.

In T356704#9559459, @jsn.sherman wrote:

and will see if I can get quibble up and running.

The more pragmatic approach would be to just unset ArticleUndelete hooks of other extensions when testing article deletion, or make Flow's hook skip when it detects it is running inside a test. Flow is on its way out, IMO it's not worth spending much time debugging it. The error is interesting (I don't see how it would be possible) but there are more interesting things than hours in the day, if it doesn't occur with any other extension I'd just leave it.

Agreed.

In T356704#9561589, @Jdforrester-WMF wrote:

In T356704#9561585, @Tgr wrote:

In T356704#9559312, @Jdforrester-WMF wrote:

I did this a year ago: 2d8f8fe3c.

Ah, right, I forgot about that.

GrowthExperiments, MassMessage, and Thanks have code which can only be tested in the presence of Flow so I guess not easily fixable. Maybe there should be a way to depend non-recursively on an extension (ie. when Jenkins checks a Thanks patch, it would install non-recursive dependencies of Thanks, but when it tests a patch from an extension that Thanks depends on, it wouldn't, and the relevant Thanks tests would mark themselves as skipped when Flow is not present), but that seems like a bigger project.

We do this for phan (as generally Foo calling Bar that implements X doesn't need to load X for internal static analysis), but not for unit/integration tests (as Foo calling Bar that calls X definitely does need X's code).

Theoretically we could do some form of automatic back-off of each pair-wise combination at test run time and see if they pass without the dependency, but yes, probably too much work.

In T356704#9559459, @jsn.sherman wrote:

and will see if I can get quibble up and running.

The more pragmatic approach would be to just unset ArticleUndelete hooks of other extensions when testing article deletion, or make Flow's hook skip when it detects it is running inside a test. Flow is on its way out, IMO it's not worth spending much time debugging it. The error is interesting (I don't see how it would be possible) but there are more interesting things than hours in the day, if it doesn't occur with any other extension I'd just leave it.

Agreed.

I believe this is the 3rd time I've encountered unreproducible CI errors involving hooks. The other 2 times involved deprecated hooks, in those cases we simply migrated to supported hooks to resolve the issue without understanding the underlying problem. I don't have an abstract interest in this error, I'd like to have more tools in my toolbox to more rapidly reproduce errors.

On to the suggested solutions. This is an area of learning for me, so I'll just post what I've looked at so far. Feel free to point me in the right direction if you are so inclined.
for us to unset other hook implementations:

• It looks like the MediaWikiIntegrationTest class provides methods for doing local hook manipulation (setTemporaryHook, clearHook).
• Our tests extend the APITestCase, so that's something I'd need to learn more about

On having flow skip undelete in tests, are you just talking about checking MW_PHPUNIT_TEST, or is there some more clever/standard way of doing it?

@Tgr, did you delete a post in this thread by accident? If so, feel free to restore it.

In T356704#9565276, @Novem_Linguae wrote:

@Tgr, did you delete a post in this thread by accident?

Umherirrender posted a better analysis between me starting to type and pressing submit, so it was redundant.

I suppose the proper fix would be to reset Flow's DI container between tests, maybe from the MediaWikiServices hook. Might not be worth the effort though.

In T356704#9565267, @jsn.sherman wrote:

On having flow skip undelete in tests, are you just talking about checking MW_PHPUNIT_TEST, or is there some more clever/standard way of doing it?

No, that's the standard way.

In T356704#9565698, @Tgr wrote:

In T356704#9565276, @Novem_Linguae wrote:

@Tgr, did you delete a post in this thread by accident?

Umherirrender posted a better analysis between me starting to type and pressing submit, so it was redundant.

I suppose the proper fix would be to reset Flow's DI container between tests, maybe from the MediaWikiServices hook. Might not be worth the effort though.

In T356704#9565267, @jsn.sherman wrote:

On having flow skip undelete in tests, are you just talking about checking MW_PHPUNIT_TEST, or is there some more clever/standard way of doing it?

No, that's the standard way.

Thanks for the guidance! I've got flow setup locally now, so I'll submit a patch for it and set it as a dependency for my PageTriage patch to restore the removed tests.

Change 1005751 had a related patch set uploaded (by Jsn.sherman; author: Jsn.sherman):

[mediawiki/extensions/Flow@master] [WIP] Skip onArticleUndelete in tests

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

Change 1005751 merged by jenkins-bot:

[mediawiki/extensions/Flow@master] Skip onArticleUndelete in tests

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

Change 997932 merged by jenkins-bot:

[mediawiki/extensions/PageTriage@master] HooksTest: Restore broken deletion tests now that Flow won't clobber them

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