Integration tests die with "HTTP requests to <url> blocked. Use MockHttpTrait." when EventBus extension is installed
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Tgr
	Dec 24 2020, 12:48 AM

Description

mwscript /vagrant/mediawiki/tests/phpunit/phpunit.php --wiki=wiki /vagrant/mediawiki/extensions/SomeUnrelatedExtension
#!/usr/bin/env php
Using PHP 7.2.31-1+0~20200514.41+debian9~1.gbpe2a56b+wmf1
PHPUnit 8.5.13 by Sebastian Bergmann and contributors.

HTTP requests to http://eventgate.local.wmftest.net:22100/v1/events blocked. Use MockHttpTrait.

Replacing the AssertionFailedError in NullHttpRequestFactory (see T262443: Do not hit actual urls during phpunit tests) with some other exception that does not get broken special handling, the trace is:

Exception from line 29 of /vagrant/mediawiki/tests/phpunit/mocks/NullMultiHttpClient.php: ...
#0 /vagrant/mediawiki/extensions/EventBus/includes/EventBus.php(244): NullMultiHttpClient->runMulti(Array, Array)
#1 /vagrant/mediawiki/extensions/EventBus/includes/EventBusHooks.php(338): MediaWiki\Extension\EventBus\EventBus->send(Array)
#2 /vagrant/mediawiki/includes/deferred/MWCallableUpdate.php(38): MediaWiki\Extension\EventBus\EventBusHooks::MediaWiki\Extension\EventBus\{closure}()
#3 /vagrant/mediawiki/includes/deferred/DeferredUpdates.php(467): MWCallableUpdate->doUpdate()
#4 /vagrant/mediawiki/includes/deferred/DeferredUpdates.php(344): DeferredUpdates::attemptUpdate(Object(MWCallableUpdate), Object(Wikimedia\Rdbms\LBFactorySimple))
#5 /vagrant/mediawiki/includes/deferred/DeferredUpdates.php(288): DeferredUpdates::run(Object(MWCallableUpdate), Object(Wikimedia\Rdbms\LBFactorySimple), Object(Monolog\Logger), Object(BufferingStatsdDataFactory), 'cli')
#6 /vagrant/mediawiki/includes/deferred/DeferredUpdates.php(190): DeferredUpdates::handleUpdateQueue(Array, 'run', 2)
#7 /vagrant/mediawiki/includes/deferred/DeferredUpdates.php(491): DeferredUpdates::doUpdates('run')
#8 /vagrant/mediawiki/includes/deferred/DeferredUpdates.php(131): DeferredUpdates::tryOpportunisticExecute('run')
#9 /vagrant/mediawiki/includes/Storage/PageUpdater.php(1319): DeferredUpdates::addUpdate(Object(AtomicSectionUpdate), 1)
#10 /vagrant/mediawiki/includes/Storage/PageUpdater.php(803): MediaWiki\Storage\PageUpdater->doCreate(Object(CommentStoreComment), Object(User), 9)
#11 /vagrant/mediawiki/includes/page/WikiPage.php(2041): MediaWiki\Storage\PageUpdater->saveRevision(Object(CommentStoreComment), 9)
#12 /vagrant/mediawiki/includes/page/WikiPage.php(1893): WikiPage->doUserEditContent(Object(WikitextContent), Object(User), Object(CommentStoreComment), 9, false, Array, 0)
#13 /vagrant/mediawiki/tests/phpunit/MediaWikiIntegrationTestCase.php(1413): WikiPage->doEditContent(Object(WikitextContent), 'UTPageSummary', 9, false, Object(User))
#14 /vagrant/mediawiki/tests/phpunit/MediaWikiIntegrationTestCase.php(421): MediaWikiIntegrationTestCase->addCoreDBData()
#15 /vagrant/mediawiki/vendor/phpunit/phpunit/src/Framework/TestSuite.php(601): MediaWikiIntegrationTestCase->run(Object(PHPUnit\Framework\TestResult))
#16 /vagrant/mediawiki/vendor/phpunit/phpunit/src/Framework/TestSuite.php(601): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
#17 /vagrant/mediawiki/vendor/phpunit/phpunit/src/TextUI/TestRunner.php(633): PHPUnit\Framework\TestSuite->run(Object(PHPUnit\Framework\TestResult))
#18 /vagrant/mediawiki/vendor/phpunit/phpunit/src/TextUI/Command.php(204): PHPUnit\TextUI\TestRunner->doRun(Object(PHPUnit\Framework\TestSuite), Array, Array, true)
#19 /vagrant/mediawiki/tests/phpunit/phpunit.php(75): PHPUnit\TextUI\Command->run(Array, true)
#20 /vagrant/mediawiki/maintenance/doMaintenance.php(106): PHPUnitMaintClass->execute()
#21 /vagrant/mediawiki/tests/phpunit/phpunit.php(134): require('/vagrant/mediaw...')
#22 /var/www/w/MWScript.php(98): require_once('/vagrant/mediaw...')
#23 {main}

Details

Subject	Repo	Branch	Lines +/-
EventLogging: Disable sending events during PHPUnit tests	mediawiki/vagrant	master	+7 -2
Suppress event logging from hook handlers when running PHPUnit tests	mediawiki/extensions/WikimediaEvents	master	+14 -4
EventBus: Disable sending events during PHPUnit tests	mediawiki/vagrant	master	+3 -2
[DNM] Workaround for CI issue T270801	mediawiki/core	master	+9 -2

Customize query in gerrit

Related Objects

Mentioned In: T265662: MockHttpTrait fails the whole test suite instead of individual test
T262443: Do not hit actual urls during phpunit tests
Mentioned Here: T267928: Configuration Modes
T262443: Do not hit actual urls during phpunit tests
T270226: HTTP request blocked: https://meta.wikimedia.org/w/api.php?action=jsonschema&revid=18910134&formatversion=2&format=json by RemoteSchema::httpGet. Use MockHttpTrait.

Event Timeline

Tgr created this task.Dec 24 2020, 12:48 AM

Restricted Application added a project: Analytics. · View Herald TranscriptDec 24 2020, 12:48 AM

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Has T270226: HTTP request blocked: https://meta.wikimedia.org/w/api.php?action=jsonschema&revid=18910134&formatversion=2&format=json by RemoteSchema::httpGet. Use MockHttpTrait. regressed again?

I think so. The trace is a bit different, that's probably because the the schema has been updated to an EventGate-based one.

In T270226#6697086, @Mholloway wrote:

The culprit was a missing check to ensure that the eventlogging service base URI was defined. A fixed version of the change is here: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/EventLogging/+/649961

...so it seems the error reoccurs when the URI is defined, which is done by Vagrant.

That task says MockHttpTrait should be used, but that's not really feasible since these are another extension's tests. A somewhat crude hack would be to have a null HTTP service or a null EventServiceClient and use that in ServiceWiring when MW_PHPUNIT_TEST is set. T267928: Configuration Modes would provide a nicer approach.

In T270801#6711245, @Tgr wrote:

I think so. The trace is a bit different, that's probably because the the schema has been updated to an EventGate-based one.

Quite a bit different, actually; T270226 was triggered in EventLogging and this is triggered in EventBus.

Restricted Application added a project: Analytics. · View Herald TranscriptDec 24 2020, 1:13 AM

Tgr renamed this task from Integration tests die with "HTTP requests to <url> blocked. Use MockHttpTrait." when EventLogging extension is installed to Integration tests die with "HTTP requests to <url> blocked. Use MockHttpTrait." when EventBus extension is installed.Dec 24 2020, 1:29 AM

I do see EventSchema in the stack trace under certain circumstances though, so I think it's actually an issue with both extensions - EventBus when there is an edit (which happens at least during PHPUnit setup) and EventLogging when the code under test logs an event.

Change 651869 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/core@master] [DNM] Workaround for CI issue T270801

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

gerritbot added a project: Patch-For-Review.Dec 24 2020, 2:04 AM

Change 651869 abandoned by Gergő Tisza:
[mediawiki/core@master] [DNM] Workaround for CI issue T270801

Reason:
workaround for local development

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

Does this fail in CI as well, or only in Vagrant? In other words, is EventGate enabling itself by default with this destination address and thus failing the expectation, or is it a case of "real" configuration from the developer's local setup leaking into the test runner?

This seems to expose a logical conflict between consistently and realism that I'm not sure how it should be solved. On the one hand there's value in detecting which extensins are installed locally and using those to load relevant tests, as well as to test against the given "environment" (db type, combination of extensions for integration/non-unit tests) and thus generally use the settings as-provided. On the other hand, we clearly also want some amount of consistently and encapsulation where tests run the same for everyone locally regardless of their environment.

If we were to skip hooks, extensions and/or local settings somehow during some or all phases of integration tests, that would presumably cause issues. E.g. if an edit is allowed to be saved early on without hooks enabled or upgrade scripts ran, then the database isn't in a valid state potentially, E.g. CheckUser data would be missing in ways that the code shouldn't have to account for. On the other hand, if we let it all get involved, then we can't mandate use of Http mock, as that would imply we don't support use of any extensions for MW that involve an HTTP service as required part of their function. The extensions' own tests could and should know how to mock it, but at other times it would need to either run normally or be disabled entirely.

I suppose one way to do this would be to limit this HttpMock restriction to the UnitTestCase. I think it would be really nice if we could keep it in the IntegrationTestCase, but I'm not sure how. It seems clearly incompatible with its own purpose. Make it opt-in or opt-out wouldn't work either (e.g. via an annotation like we do for @group Database), since it'd be impossible to set that right in a meaningful way as the owner/writer of a test case can't be expected to cover what every possible valid/supported extension might do during a hook that runs during its integration work. E.g. anything with an edit or other major action would need to allow HTTP just in case something like EventLogging is configured and enabled by the developer's local settings. The same canundrum applies to the database as well, although we're lucky there in that it's very rare for a hook write to the database for tested actions that themselves don't already use the database, at least we haven't run into that yet afaik.

• Mholloway mentioned this in T262443: Do not hit actual urls during phpunit tests.Jan 4 2021, 3:55 PM

• fdans moved this task from Incoming to Event Platform on the Analytics board.Jan 4 2021, 5:29 PM

• Mholloway mentioned this in T265662: MockHttpTrait fails the whole test suite instead of individual test.Jan 4 2021, 5:36 PM

Change 655124 had a related patch set uploaded (by Mholloway; owner: Michael Holloway):
[mediawiki/vagrant@master] Disable sending events during PHPUnit tests

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

In T270801#6711308, @Krinkle wrote:

Does this fail in CI as well, or only in Vagrant? In other words, is EventGate enabling itself by default with this destination address and thus failing the expectation, or is it a case of "real" configuration from the developer's local setup leaking into the test runner?

Vagrant only; by default EventGate doesn't do anything, it has an enable flag of sorts. @Mholloway's patch unsets the flag for tests.

Change 655124 merged by jenkins-bot:
[mediawiki/vagrant@master] EventBus: Disable sending events during PHPUnit tests

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

The EventLogging extension also has the same problem, although it doesn't completely break the test runner so it's less annoying. Not sure if that's configuration-dependent.

1) GrowthExperiments\Tests\MentorPageMentorManagerTest::testGetMentorUserNew
HTTP request blocked: https://meta.wikimedia.org/w/api.php?action=jsonschema&revid=14208837&formatversion=2&format=json by RemoteSchema::httpGet. Use MockHttpTrait.

/vagrant/mediawiki/tests/phpunit/mocks/NullHttpRequestFactory.php:43
/vagrant/mediawiki/includes/http/HttpRequestFactory.php:189
/vagrant/mediawiki/includes/http/HttpRequestFactory.php:212
/vagrant/mediawiki/extensions/EventLogging/includes/RemoteSchema.php:108
/vagrant/mediawiki/extensions/EventLogging/includes/RemoteSchema.php:57
/vagrant/mediawiki/extensions/EventLogging/includes/EventLogging.php:195
/vagrant/mediawiki/extensions/EventLogging/includes/EventLogging.php:84
/vagrant/mediawiki/extensions/WikimediaEvents/includes/WikimediaEventsHooks.php:299
/vagrant/mediawiki/includes/deferred/MWCallableUpdate.php:38
/vagrant/mediawiki/includes/deferred/DeferredUpdates.php:467
/vagrant/mediawiki/includes/deferred/DeferredUpdates.php:344
/vagrant/mediawiki/includes/deferred/DeferredUpdates.php:278
/vagrant/mediawiki/includes/deferred/DeferredUpdates.php:194
/vagrant/mediawiki/includes/deferred/DeferredUpdates.php:491
/vagrant/mediawiki/maintenance/includes/Maintenance.php:698
/vagrant/mediawiki/includes/libs/rdbms/database/Database.php:4241
/vagrant/mediawiki/includes/libs/rdbms/database/Database.php:4610
/vagrant/mediawiki/includes/libs/rdbms/database/Database.php:4371
/vagrant/mediawiki/includes/libs/rdbms/database/DBConnRef.php:68
/vagrant/mediawiki/includes/libs/rdbms/database/DBConnRef.php:629
/vagrant/mediawiki/includes/Storage/PageUpdater.php:1132
/vagrant/mediawiki/includes/Storage/PageUpdater.php:801
/vagrant/mediawiki/includes/page/WikiPage.php:2041
/vagrant/mediawiki/includes/page/WikiPage.php:1893
/vagrant/mediawiki/tests/phpunit/MediaWikiIntegrationTestCase.php:1354
/vagrant/mediawiki/extensions/GrowthExperiments/tests/phpunit/integration/MentorPageMentorManagerTest.php:72
/vagrant/mediawiki/tests/phpunit/MediaWikiIntegrationTestCase.php:437
/vagrant/mediawiki/tests/phpunit/phpunit.php:75
/vagrant/mediawiki/maintenance/doMaintenance.php:106
/vagrant/mediawiki/tests/phpunit/phpunit.php:134

(The actual logging code that breaks is in WikimediaEvents, and it fires on the user's fifth edit in a calendar month, so this is probably somethat nondeterministic.)

Looking at the EventLogging Vagrant config, it doesn't look like any relevant configuration flag is being set to a non-default value.

It looks like most if not all of the past and remaining offenders involve events created by hook handlers in WikimediaEventsHooks. (I tried to find some remaining failures last Friday and couldn't because I didn't have wikimediaevents enabled until after looking at your stack trace this morning.) Maybe the best thing to do here is to suppress them at the source if MW_PHPUNIT_TEST is set.

Change 655456 had a related patch set uploaded (by Mholloway; owner: Michael Holloway):
[mediawiki/extensions/WikimediaEvents@master] Suppress event logging from hook handlers when running PHPUnit tests

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

Change 655488 had a related patch set uploaded (by Mholloway; owner: Michael Holloway):
[mediawiki/vagrant@master] EventLogging: Disable sending events during PHPUnit tests

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

Change 655456 abandoned by Mholloway:
[mediawiki/extensions/WikimediaEvents@master] Suppress event logging from hook handlers when running PHPUnit tests

Reason:
See I64f71e