Page MenuHomePhabricator

CI failing in CirrusSearch tests with “Undefined index: REQUEST_TIME_FLOAT”
Closed, ResolvedPublic

Description

Build #48760 (for I2c929f8ef6) for repo mediawiki/extensions/OAuth (and others) are failing with errors from CirrusSearch, like the following:

12:11:18 2) CirrusSearch\Test\RequestLoggerTest::testHasQueryLogs
12:11:18 Undefined index: REQUEST_TIME_FLOAT
12:11:18 
12:11:18 /workspace/src/includes/WebRequest.php:94
12:11:18 /workspace/src/includes/context/RequestContext.php:120
12:11:18 /workspace/src/extensions/CirrusSearch/includes/Util.php:423
12:11:18 /workspace/src/extensions/CirrusSearch/includes/RequestLogger.php:128
12:11:18 /workspace/src/extensions/CirrusSearch/tests/phpunit/RequestLoggerTest.php:53
12:11:18 /workspace/src/tests/phpunit/MediaWikiTestCase.php:427
12:11:18 /workspace/src/tests/phpunit/phpunit.php:90
12:11:18 /workspace/src/maintenance/doMaintenance.php:98
12:11:18 /workspace/src/tests/phpunit/phpunit.php:129

Details

Related Gerrit Patches:
mediawiki/extensions/OAuth : masterUse RequestContext::resetMain() instead of removing everything
mediawiki/extensions/OAuth : masterBack up globals before OAuthRequest tests

Event Timeline

Restricted Application added a project: Discovery-Search. · View Herald TranscriptMay 14 2019, 10:23 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Ladsgroup triaged this task as High priority.May 15 2019, 9:03 AM
Ladsgroup added a subscriber: Ladsgroup.

It's almost UBN! We can't merge anything in a couple of repos

REQUEST_TIME_FLOAT has been around since 5.4.0, what is running that doesn't have it?

Smalyshev added a subscriber: Smalyshev.EditedMay 16 2019, 2:42 PM

And the code failing is actually in core, it's just triggered by CirrusSearch test. I suspect it may be not 5.4 issue but something expects server variables being there while running on CLI.

Update: CLI has REQUEST_TIME_FLOAT too, so I wonder how it's missing. Maybe it's hhvm?

The code has existed since 2016, in https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/304465/. Anything that constructed a WebRequest object would have triggered this code, so I'm quite surprised if this is a new problem. I would more suspect that something in CI changed rather than the code.

And the code failing is actually in core, it's just triggered by CirrusSearch test. I suspect it may be not 5.4 issue but something expects server variables being there while running on CLI.
Update: CLI has REQUEST_TIME_FLOAT too, so I wonder how it's missing. Maybe it's hhvm?

hhvm cli also has REQUEST_TIME_FLOAT.

ebernhardson@mwdebug1001:~$ echo '<?php var_dump($_SERVER["REQUEST_TIME_FLOAT"]);' | hhvm --php
float(1558281568.8856)

I suspect something at run-time is mocking $_SERVER in a way that blows away all its keys, and is not restoring the array afterwards.

Krinkle raised the priority of this task from High to Unbreak Now!.May 20 2019, 11:55 PM

It's almost UBN! We can't merge anything in a couple of repos

That's UBN.

Restricted Application added a subscriber: Liuxinyu970226. · View Herald TranscriptMay 20 2019, 11:55 PM
Krinkle updated the task description. (Show Details)May 20 2019, 11:59 PM

Appears to be caused by OAuth itself:

OAuth/OAuthTestUtils
class OAuthTestUtils {
	private static function reset_request_vars() {
		$_SERVER = array();

This code has somehow survived for 6 years without causing issues. Likely because OAUth is not part of the shared gate, and is only tested with "Echo" installed. Thus no other extensions are impacted by it.

The CirrusSearch test triggers the (otherwise fine) dependency as follows:

CirrusSearch/Util
		$request = \RequestContext::getMain()->getRequest();

And in core:

MediaWikiTestCase
	public static function resetNonServiceCaches() {
		
		RequestContext::resetMain();
	}
	protected function setUp() {
		
		self::resetNonServiceCaches();
	}

I don't know why it is that these passed before and are failing now. The code has been in OAuth for years. The test has been in CirrusSearch for a year. And CirrusSearch has been in the shared gate job for years as well.

@LucasWerkmeister Is this causing commits in other repos to fail? I would expect it to be limited to OAuth itself only, and thus does not require tracking via the Shared-Build-Failure if the case.

Krinkle lowered the priority of this task from Unbreak Now! to High.May 21 2019, 12:23 AM

Change 511653 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/OAuth@master] Use RequestContext::resetMain() instead of removing everything

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

I don’t remember seeing it in other repos – I think I just added the tag assuming that it wouldn’t be restricted to OAuth if it looked like a CirrusSearch failure. Sorry for the confusion.

Change 513746 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/extensions/OAuth@master] Back up globals before OAuthRequest tests

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

Change 513746 merged by jenkins-bot:
[mediawiki/extensions/OAuth@master] Back up globals before OAuthRequest tests

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

LucasWerkmeister closed this task as Resolved.Jun 3 2019, 11:55 AM
LucasWerkmeister assigned this task to Tgr.