Change 658648 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/core@master] Call TestCase::fail() more carefully in MockHttpTrait

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

Change 658649 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] DNM: empty change to test CI

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

I’m not sure why this only started failing now – which part of that chain changed recently – but I think the fix ought to be that MockHttpTrait passes a callback into the mocks which wraps TestCase::fail() but discards any arguments with which it was called.

Wait, but that would still make the test fail, just differently… I’m confused now.

In T272998#6778032, @Lucas_Werkmeister_WMDE wrote:

I’m not sure why this only started failing now – which part of that chain changed recently – but I think the fix ought to be that MockHttpTrait passes a callback into the mocks which wraps TestCase::fail() but discards any arguments with which it was called.

Wait, but that would still make the test fail, just differently… I’m confused now.

Yeah, the test still fails in that empty change to test CI, just with a better error message – instead of a TypeError it’s now “method should not be called”.

I can reproduce the error locally when loading the EventBus extension, and the error still remains if I revert MediaWiki core, Wikibase, and EventBus to ten commits earlier. (In the case of MediaWiki, even fifty commits.) But unlike in T269608 it’s no mystery why the error only started happening now in CI – there was an integration/config change yesterday which added EventBus as a dependency of EventLogging (tagged with T272863), and I’d guess that this transitively pulled EventBus into the Wikibase tests, causing the error.

I’m not sure how to properly fix this error. I’m guessing that pulling EventBus out of the CI config again isn’t the best solution (but CC @Mholloway, @Ottomata), but I don’t know enough about it to say whether that call to createMultiClient() can / should be avoided. For now we can skip the failing Wikibase test, but that’s not a complete solution.

Change 658949 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Temporarily skip SetClaimTest::testFederatedPropertiesFailure

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

Change 658649 abandoned by Lucas Werkmeister (WMDE):
[mediawiki/extensions/Wikibase@master] DNM: empty change to test CI

Reason:
no longer needed

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

I tried deferring the createMultiClient() call a bit in EventBus –

diff --git a/ServiceWiring.php b/ServiceWiring.php
index 67912b5..9883d78 100644
--- a/ServiceWiring.php
+++ b/ServiceWiring.php
@@ -24,7 +24,7 @@
 			),
 			$streamConfigs,
 			$services->get( 'EventBus.EventFactory' ),
-			$services->getHttpRequestFactory()->createMultiClient(),
+			$services->getHttpRequestFactory(),
 			LoggerFactory::getInstance( 'EventBus' )
 		);
 	},
diff --git a/includes/EventBusFactory.php b/includes/EventBusFactory.php
index 85cf582..6ef715e 100644
--- a/includes/EventBusFactory.php
+++ b/includes/EventBusFactory.php
@@ -25,7 +25,7 @@
 use InvalidArgumentException;
 use MediaWiki\Config\ServiceOptions;
 use Mediawiki\Extension\EventStreamConfig\StreamConfigs;
-use MultiHttpClient;
+use MediaWiki\Http\HttpRequestFactory;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -70,8 +70,8 @@ class EventBusFactory {
 	/** @var EventFactory */
 	private $eventFactory;
 
-	/** @var MultiHttpClient */
-	private $http;
+	/** @var HttpRequestFactory */
+	private $httpRequestFactory;
 
 	/** @var LoggerInterface */
 	private $logger;
@@ -83,14 +83,14 @@ class EventBusFactory {
 	 * @param ServiceOptions $options
 	 * @param StreamConfigs|null $streamConfigs
 	 * @param EventFactory $eventFactory
-	 * @param MultiHttpClient $http
+	 * @param HttpRequestFactory $httpRequestFactory
 	 * @param LoggerInterface $logger
 	 */
 	public function __construct(
 		ServiceOptions $options,
 		?StreamConfigs $streamConfigs,
 		EventFactory $eventFactory,
-		MultiHttpClient $http,
+		HttpRequestFactory $httpRequestFactory,
 		LoggerInterface $logger
 	) {
 		$options->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS );
@@ -101,7 +101,7 @@ public function __construct(
 		$this->maxBatchByteSize = $options->get( 'EventBusMaxBatchByteSize' );
 		$this->streamConfigs = $streamConfigs;
 		$this->eventFactory = $eventFactory;
-		$this->http = $http;
+		$this->httpRequestFactory = $httpRequestFactory;
 		$this->logger = $logger;
 	}
 
@@ -138,7 +138,7 @@ public function getInstance( string $eventServiceName ) : EventBus {
 
 		if ( !array_key_exists( $eventServiceName, $this->eventBusInstances ) ) {
 			$this->eventBusInstances[$eventServiceName] = new EventBus(
-				$this->http,
+				$this->httpRequestFactory->createMultiClient(),
 				$this->enableEventBus,
 				$this->eventFactory,
 				$url,
diff --git a/tests/phpunit/unit/EventBusFactoryTest.php b/tests/phpunit/unit/EventBusFactoryTest.php
index d2b5f3f..9dc8514 100644
--- a/tests/phpunit/unit/EventBusFactoryTest.php
+++ b/tests/phpunit/unit/EventBusFactoryTest.php
@@ -1,8 +1,10 @@
 <?php
+
 use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Extension\EventBus\EventBusFactory;
 use MediaWiki\Extension\EventBus\EventFactory;
 use MediaWiki\Extension\EventStreamConfig\StreamConfigs;
+use MediaWiki\Http\HttpRequestFactory;
 use Psr\Log\NullLogger;
 use Wikimedia\TestingAccessWrapper;
 
@@ -48,7 +50,7 @@ private function getEventBusFactory( array $mwConfig ) : EventBusFactory {
 			),
 			$streamConfigs,
 			$this->createNoOpMock( EventFactory::class ),
-			$this->createNoOpMock( MultiHttpClient::class ),
+			$this->createNoOpMock( HttpRequestFactory::class ),
 			new NullLogger()
 		);
 	}

– but it doesn’t work. The hook handler that’s being called accesses not just the EventBusFactory, but a specific EventBus instance, so as long as the EventBus gets a MultiHttpClient injected the test will still fail.

Maybe MockHttpTrait should allow calls to createMultiClient() (and possibly createGuzzleClient(), which looks similar), and only fail the test when an actual request is launched (run(), runMulti())?

Change 658951 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/core@master] Allow more calls in MockHttpTrait

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

In T272998#6779771, @Lucas_Werkmeister_WMDE wrote:

Maybe MockHttpTrait should allow calls to createMultiClient() (and possibly createGuzzleClient(), which looks similar), and only fail the test when an actual request is launched (run(), runMulti())?

That seems to work, see the above MediaWiki change. (I’ve tested it locally, but if someone wants to be convinced it works in CI too, we could upload another no-op change in Wikibase to verify it.)

Nice Lucas.

EventBus should be added to the wmf-quibble* jobs as well (T272863) and PHPUnit tests currently fail because of that string/array mismatch.

In T272998#6780125, @hashar wrote:

EventBus should be added to the wmf-quibble* jobs as well (T272863) and PHPUnit tests currently fail because of that string/array mismatch.

Alright, then we should probably go for the MediaWiki core solution, if the Wikibase test isn’t the only one that would need to be skipped.

Edit: nevermind, the failing test in that build is the same Wikibase test.

In T272998#6779816, @Lucas_Werkmeister_WMDE wrote:

(I’ve tested it locally, but if someone wants to be convinced it works in CI too, we could upload another no-op change in Wikibase to verify it.)

I’ve added a Depends-On line to this libup change which had been failing with the same error earlier – now it passes \o/

Change 658951 merged by jenkins-bot:
[mediawiki/core@master] Allow more calls in MockHttpTrait

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

Change 658949 abandoned by Lucas Werkmeister (WMDE):
[mediawiki/extensions/Wikibase@master] Temporarily skip SetClaimTest::testFederatedPropertiesFailure

Reason:
no longer needed

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

Change 658648 merged by jenkins-bot:
[mediawiki/core@master] Call TestCase::fail() more carefully in MockHttpTrait

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

Change 662716 had a related patch set uploaded (by Ottomata; owner: Ottomata):
[operations/puppet@production] Add an MW local envoy listener for eventgate-analytics-external

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

Change 662719 had a related patch set uploaded (by Ottomata; owner: Ottomata):
[operations/mediawiki-config@master] Add eventgate-analytics-external to (Production|Labs)Services.php

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

Change 662719 merged by Ottomata:
[operations/mediawiki-config@master] Add eventgate-analytics-external to (Production|Labs)Services.php

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

Change 662746 had a related patch set uploaded (by Ottomata; owner: Ottomata):
[operations/mediawiki-config@master] Add eventgate-analytics-external to wgEventServices

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

Change 662746 merged by Ottomata:
[operations/mediawiki-config@master] Add eventgate-analytics-external to wgEventServices

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

Mentioned in SAL (#wikimedia-operations) [2021-02-08T17:20:07Z] <otto@deploy1001> Synchronized wmf-config/LabsServices.php: LabsServices - Add eventgate-analytics-external - T272998 (duration: 01m 08s)

Change 662716 merged by Ottomata:
[operations/puppet@production] Add an MW local envoy listener for eventgate-analytics-external

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