Page MenuHomePhabricator
Create Task
Maniphest T232691

Unit tests in mediawiki-config repo don't pass under PHP72, only HHVM
Closed, ResolvedPublic
Actions

Description

00:00:28.408 PHPUnit 4.8.36 by Sebastian Bergmann and contributors.
00:00:28.408 
00:00:28.408 Runtime:	PHP 7.2.16-1+0~20190307202415.17+stretch~1.gbpa7be82+wmf1
00:00:28.408 Configuration:	/src/phpunit.xml
00:00:28.408 
00:00:28.417 .............................................................   61 / 1161 (  5%)
00:00:28.434 .............................................................  122 / 1161 ( 10%)
00:00:28.451 .............................................................  183 / 1161 ( 15%)
00:00:28.468 .............................................................  244 / 1161 ( 21%)
00:00:28.480 .............................................................  305 / 1161 ( 26%)
00:00:28.498 .............................................................  366 / 1161 ( 31%)
00:00:28.519 .............................................................  427 / 1161 ( 36%)
00:00:28.537 .............................................................  488 / 1161 ( 42%)
00:00:28.555 .............................................................  549 / 1161 ( 47%)
00:00:28.572 .............................................................  610 / 1161 ( 52%)
00:00:28.600 .............................................................  671 / 1161 ( 57%)
00:00:28.608 .............................................................  732 / 1161 ( 63%)
00:00:28.614 .............................................................  793 / 1161 ( 68%)
00:00:28.638 ............................................................S  854 / 1161 ( 73%)
00:00:29.266 .......RRRR.RRRR.............................................  915 / 1161 ( 78%)
00:00:29.435 .............................................................  976 / 1161 ( 84%)
00:00:29.448 ............................................................. 1037 / 1161 ( 89%)
00:00:29.467 ............................................................. 1098 / 1161 ( 94%)
00:00:29.489 ............................................................. 1159 / 1161 ( 99%)
00:00:29.537 ..
00:00:29.538 
00:00:29.538 Time: 1.22 seconds, Memory: 20.00MB
00:00:29.538 
00:00:29.538 There were 8 risky tests:
00:00:29.538 
00:00:29.538 1) DbconfigTest::testSectionLoadsInHostsbyname with data set #0 ('production', 'eqiad', 'eqiad')
00:00:29.539 --- Global variables before the test
00:00:29.539 +++ Global variables after the test
00:00:29.539 @@ @@
00:00:29.539      )
00:00:29.539 -    'wmfDatacenter' => 'no-cluster-configured!'
00:00:29.539 +    'wmfDatacenter' => 'eqiad'
00:00:29.539      'wmfRealm' => 'production'
00:00:29.539      'wmgRealm' => 'production'
00:00:29.540      '__PHPUNIT_CONFIGURATION_FILE' => '/src/phpunit.xml'
00:00:29.540      '__PHPUNIT_BOOTSTRAP' => '/src/./tests/bootstrap.php'
00:00:29.540  )
00:00:29.540 
00:00:29.540 2) DbconfigTest::testSectionLoadsInHostsbyname with data set #1 ('production', 'eqiad', 'codfw')
00:00:29.540 --- Global variables before the test
00:00:29.541 +++ Global variables after the test
00:00:29.541 @@ @@
00:00:29.541      )
00:00:29.541 -    'wmfDatacenter' => 'no-cluster-configured!'
00:00:29.541 +    'wmfDatacenter' => 'eqiad'
00:00:29.541      'wmfRealm' => 'production'
00:00:29.541      'wmgRealm' => 'production'
00:00:29.541      '__PHPUNIT_CONFIGURATION_FILE' => '/src/phpunit.xml'
00:00:29.541      '__PHPUNIT_BOOTSTRAP' => '/src/./tests/bootstrap.php'
00:00:29.542  )
00:00:29.542 
00:00:29.542 3) DbconfigTest::testSectionLoadsInHostsbyname with data set #2 ('production', 'codfw', 'eqiad')
00:00:29.542 --- Global variables before the test
00:00:29.542 +++ Global variables after the test
00:00:29.543 @@ @@
00:00:29.543      )
00:00:29.543 -    'wmfDatacenter' => 'no-cluster-configured!'
00:00:29.543 +    'wmfDatacenter' => 'codfw'
00:00:29.543      'wmfRealm' => 'production'
00:00:29.543      'wmgRealm' => 'production'
00:00:29.543      '__PHPUNIT_CONFIGURATION_FILE' => '/src/phpunit.xml'
00:00:29.544      '__PHPUNIT_BOOTSTRAP' => '/src/./tests/bootstrap.php'
00:00:29.544  )
00:00:29.544 
00:00:29.544 4) DbconfigTest::testSectionLoadsInHostsbyname with data set #3 ('production', 'codfw', 'codfw')
00:00:29.544 --- Global variables before the test
00:00:29.544 +++ Global variables after the test
00:00:29.544 @@ @@
00:00:29.545      )
00:00:29.545 -    'wmfDatacenter' => 'no-cluster-configured!'
00:00:29.545 +    'wmfDatacenter' => 'codfw'
00:00:29.545      'wmfRealm' => 'production'
00:00:29.545      'wmgRealm' => 'production'
00:00:29.545      '__PHPUNIT_CONFIGURATION_FILE' => '/src/phpunit.xml'
00:00:29.545      '__PHPUNIT_BOOTSTRAP' => '/src/./tests/bootstrap.php'
00:00:29.546  )
00:00:29.546 
00:00:29.546 5) DbconfigTest::testDbAssignedToAnExistingCluster with data set #0 ('production', 'eqiad', 'eqiad')
00:00:29.546 --- Global variables before the test
00:00:29.546 +++ Global variables after the test
00:00:29.546 @@ @@
00:00:29.547      )
00:00:29.547 -    'wmfDatacenter' => 'no-cluster-configured!'
00:00:29.547 +    'wmfDatacenter' => 'eqiad'
00:00:29.547      'wmfRealm' => 'production'
00:00:29.547      'wmgRealm' => 'production'
00:00:29.547      '__PHPUNIT_CONFIGURATION_FILE' => '/src/phpunit.xml'
00:00:29.547      '__PHPUNIT_BOOTSTRAP' => '/src/./tests/bootstrap.php'
00:00:29.548  )
00:00:29.548 
00:00:29.548 6) DbconfigTest::testDbAssignedToAnExistingCluster with data set #1 ('production', 'eqiad', 'codfw')
00:00:29.548 --- Global variables before the test
00:00:29.548 +++ Global variables after the test
00:00:29.548 @@ @@
00:00:29.548      )
00:00:29.548 -    'wmfDatacenter' => 'no-cluster-configured!'
00:00:29.549 +    'wmfDatacenter' => 'eqiad'
00:00:29.549      'wmfRealm' => 'production'
00:00:29.549      'wmgRealm' => 'production'
00:00:29.549      '__PHPUNIT_CONFIGURATION_FILE' => '/src/phpunit.xml'
00:00:29.549      '__PHPUNIT_BOOTSTRAP' => '/src/./tests/bootstrap.php'
00:00:29.549  )
00:00:29.549 
00:00:29.549 7) DbconfigTest::testDbAssignedToAnExistingCluster with data set #2 ('production', 'codfw', 'eqiad')
00:00:29.550 --- Global variables before the test
00:00:29.550 +++ Global variables after the test
00:00:29.550 @@ @@
00:00:29.550      )
00:00:29.550 -    'wmfDatacenter' => 'no-cluster-configured!'
00:00:29.550 +    'wmfDatacenter' => 'codfw'
00:00:29.551      'wmfRealm' => 'production'
00:00:29.551      'wmgRealm' => 'production'
00:00:29.551      '__PHPUNIT_CONFIGURATION_FILE' => '/src/phpunit.xml'
00:00:29.551      '__PHPUNIT_BOOTSTRAP' => '/src/./tests/bootstrap.php'
00:00:29.551  )
00:00:29.551 
00:00:29.551 8) DbconfigTest::testDbAssignedToAnExistingCluster with data set #3 ('production', 'codfw', 'codfw')
00:00:29.552 --- Global variables before the test
00:00:29.552 +++ Global variables after the test
00:00:29.552 @@ @@
00:00:29.552      )
00:00:29.552 -    'wmfDatacenter' => 'no-cluster-configured!'
00:00:29.552 +    'wmfDatacenter' => 'codfw'
00:00:29.553      'wmfRealm' => 'production'
00:00:29.553      'wmgRealm' => 'production'
00:00:29.553      '__PHPUNIT_CONFIGURATION_FILE' => '/src/phpunit.xml'
00:00:29.553      '__PHPUNIT_BOOTSTRAP' => '/src/./tests/bootstrap.php'
00:00:29.553  )
00:00:29.553 
00:00:29.553 --
00:00:29.553 
00:00:29.553 There was 1 skipped test:
00:00:29.553 
00:00:29.553 1) CirrusTest::testShardAndReplicasCountsAreSane
00:00:29.554 Test for CirrusTest::testShardAndReplicasCountsAreSane skipped by data provider
00:00:29.554 
00:00:29.554 OK, but incomplete, skipped, or risky tests!
00:00:29.554 Tests: 1161, Assertions: 18915, Skipped: 1, Risky: 8.
00:00:29.555 PHP Fatal error:  Uncaught Exception: WgConfTestCase: setGlobals() used without restoreGlobals().
00:00:29.555 Mangled globals:
00:00:29.555 array (
00:00:29.555   'wmfRealm' => 'production',
00:00:29.555   'wmfDatacenter' => 'no-cluster-configured!',
00:00:29.555 )Created globals:
00:00:29.555 array (
00:00:29.555 ) in /src/tests/WgConfTestCase.php:74
00:00:29.555 Stack trace:
00:00:29.555 #0 [internal function]: WgConfTestCase->__destruct()
00:00:29.556 #1 {main}
00:00:29.556   thrown in /src/tests/WgConfTestCase.php on line 74
00:00:29.556 Script phpunit handling the test event returned with error code 255
00:00:29.954

Details

SubjectRepoBranchLines +/-
[cirrus] fix cirrusTest.phpoperations/mediawiki-configmaster+1 -1
tests: Remove dbconfigTest.phpoperations/mediawiki-configmaster+7 -116
tests/WgConfTestCase: HACK: Downgrade throw to echooperations/mediawiki-configmaster+2 -9
tests: Fix "Deprecated: The each() function is deprecated" from timelineTest.php:32operations/mediawiki-configmaster+1 -1
tests: Skip all dbConfigTest testsr when not on HHVMoperations/mediawiki-configmaster+8 -0
Customize query in gerrit

Event Timeline

Jdforrester-WMF created this task.Sep 12 2019, 12:02 AM
Restricted Application added a project: Discovery-Search. · View Herald TranscriptSep 12 2019, 12:02 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Jdforrester-WMF removed projects: CirrusSearch, Discovery-Search.Sep 12 2019, 12:16 AM

Not the CirrusSearch tests, just the DbconfigTest ones.

gerritbot added a comment.Sep 12 2019, 12:20 AM

Change 535973 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[operations/mediawiki-config@master] tests: Skip all dbConfigTest testsr when not on HHVM

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

gerritbot added a project: Patch-For-Review.Sep 12 2019, 12:20 AM
Jdforrester-WMF added a subscriber: hashar.Sep 12 2019, 12:21 AM

Antoine, you wrote these tests back in 2012; did we ever test them on real PHP? I've skipped them on PHP72 for now, but we need to properly fix them. :-)

gerritbot added a comment.Sep 12 2019, 12:22 AM

Change 535973 merged by jenkins-bot:
[operations/mediawiki-config@master] tests: Skip all dbConfigTest testsr when not on HHVM

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

Jdforrester-WMF triaged this task as High priority.Sep 12 2019, 12:26 AM
Jdforrester-WMF removed a project: Patch-For-Review.
Krinkle subscribed.Sep 12 2019, 4:15 PM
gerritbot added a comment.Sep 12 2019, 4:41 PM

Change 536232 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] tests: Fix "Deprecated: The each() function is deprecated" from timelineTest.php:32

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

gerritbot added a project: Patch-For-Review.Sep 12 2019, 4:41 PM
gerritbot added a comment.Sep 12 2019, 5:30 PM

Change 536232 merged by jenkins-bot:
[operations/mediawiki-config@master] tests: Fix "Deprecated: The each() function is deprecated" from timelineTest.php:32

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

Maintenance_bot removed a project: Patch-For-Review.Sep 12 2019, 6:10 PM
Krinkle added a comment.EditedSep 12 2019, 6:47 PM
CirussSearch test warnings

It is by a lot of luck and errors cancelling each other out that this test is passing. Below a simplified version of wmf-config/tests/cirussTest.php:

	/** @return array Test cases for testShardAndReplicasCountsAreSane */
	public function providePerClusterShardsAndReplicas() {
		$wgConf = $this->loadWgConf( 'unittest' );
		$shards = $wgConf->settings['wgCirrusSearchShardCount'];
		$replicas = $wgConf->settings['wgCirrusSearchReplicas'];
		$maxShardPerNode = $wgConf->settings['wgCirrusSearchMaxShardsPerNode'];
		$wikis = array_merge(
			array_keys( $shards ),
			array_keys( $replicas ),
			array_keys( $maxShardPerNode )
		);
		foreach ( $wikis as $idx => $wiki ) {
			if ( $wiki[0] === '+' ) {
				$wikis[$idx] = substr( $wiki, 1 );
			}
		}
		$wikis = array_unique( $wikis );

		// Replace wgConf with a reduced view of the variables we want to extract
		$wgConf->settings = [ … ];
		$tests = [];
		foreach ( $wikis as $wiki ) {
			$config = $wgConf->getAll( … );
			// …
			$tests[ … ] = [
				…,
				self::resolveClusterConfig( $config['shards'], … ),
				self::resolveClusterConfig( $config['replicas'], … ),
				self::resolveClusterConfig( $config['max_shards_per_node'], … ),
			];
		}

		return $tests;
	}
PHPUnit output
PHP Notice: Undefined index: wgCirrusSearchShardCount in …/tests/cirrusTest.php on line 196

PHP Warning: array_keys() expects parameter 1 to be array, null given in …/tests/cirrusTest.php on line 200

PHP Warning: array_merge(): Argument #1 is not an array in …/tests/cirrusTest.php on line 202

PHP Warning: Invalid argument supplied for foreach() in …/tests/cirrusTest.php on line 204

PHP Warning: array_unique() expects parameter 1 to be array, null given in …/tests/cirrusTest.php on line 209


1) CirrusTest::testShardAndReplicasCountsAreSane
Test for CirrusTest::testShardAndReplicasCountsAreSane skipped by data provider

OK, but has skipped tests!

So the question is, how come through all this code, it still manages to return a valid data provider?

First, wgCirrusSearchShardCount. This key is only set by CirrusSearch-production.php, which is not loaded when loading wgConfig for realm='unittest'. It probably needs a default or fallback somewhere. With the variable being undefined, PHP silently continues and gives it it the null value.

The expected value for this variable is a wgConf-compatible array where the top level are keys like default, wikipedia, or enwiki. The further code is expecting this, and takes the keys of each of the three configuration variables to build a list of wikis to run a unit test for.

$shards = $wgConf->settings['wgCirrusSearchShardCount'];
# ^ Actual: null
# ^ Expected: [ 'default' => …, 'enwiki' => …, … ]

Then we have wgCirrusSearchReplicas and wgCirrusSearchMaxShardsPerNode. These are both set in InitialiseSettings as expected and work fine:

$replicas = $wgConf->settings['wgCirrusSearchReplicas'];
# ^ [ 'default' => …, 'enwiki' => …, … ]
$replicas = $wgConf->settings['wgCirrusSearchReplicas'];
# ^ [ 'default' => …, 'commonswiki' => …, … ]

Then we get to the extracting of keys to build a list of wiki IDs the test suite will try to validate.

$wikis = array_merge(
	array_keys( $replicas ),
	          # ^ [ 'default' => …, 'enwiki' => … ]
	array_keys( $shards ),
	          # ^ null
	array_keys( $maxShardPerNode )
	          # ^ [ 'default' => …, 'commonswiki' => … ]
);

So, calling array_keys( null ) is garage-in and garbage-out is to be expected. One might expect null to cast to an empty array and thus "all keys of the array" would yield another empty array . Or one might expect null to cast to an array containing null as its value [ null ], of which the keys would be [ 0 ]. Instead, calling array_keys ( … ) : array with null returns... null.

$wikis = array_merge(
   [ 'default', 'enwii' ],      # array_keys( $replicas ),
   null,                        # array_keys( $shards ),
   [ 'default', 'commonswiki' ] # array_keys( $maxShardPerNode )
);

So what's array_merge() going to do? Well, it took is confidently documented as always returning an array. And you guessed it right - it discards all other parameters and lets the null take over (like NaN in math).

$wikis = null; # array_merge( [ 'default', 'enwiki' ],  null,  [ 'default', 'commonswiki' ] );

Then, lastly it is going to map this array to an array of test cases:

$tests = [];
foreach ( $wikis as $wiki ) {
	$tests[ … ] = [ …, self::resolveClusterConfig( $config['shards'], … ), … ]
}
return $tests;

And here foreach() conveniently tolerates null as an empty iterable and thus skips the loops and lets PHPUnit get an empty array of no test cases to cover, and marks the test as a Success!.

Krinkle added a comment.Sep 12 2019, 6:56 PM

I tried to hot-fix it by adding tolerance for this undefined variable:

- $shards = $wgConf->settings['wgCirrusSearchShardCount'];
+ $shards = $wgConf->settings['wgCirrusSearchShardCount'] ?? [];

Unfortunately, while that avoids all the cascading failures described above, it then leads to a sea of:

PHP Notice:  Undefined index: shards in …/tests/cirrusTest.php on line 233
PHP Notice:  Undefined index: shards in …/tests/cirrusTest.php on line 233
PHP Notice:  Undefined index: shards in …/tests/cirrusTest.php on line 233
PHP Notice:  Undefined index: shards in …/tests/cirrusTest.php on line 233
…

This is due to wgConf not setting the global unless it is given an array with at least the key 'default' in it.

- $shards = $wgConf->settings['wgCirrusSearchShardCount'];
- $shards = $wgConf->settings['wgCirrusSearchShardCount'] ?? [];
+ $shards = $wgConf->settings['wgCirrusSearchShardCount'] ?? [ 'default' => null ];

This leads to the same failure.

- $shards = $wgConf->settings['wgCirrusSearchShardCount'];
- $shards = $wgConf->settings['wgCirrusSearchShardCount'] ?? [];
- $shards = $wgConf->settings['wgCirrusSearchShardCount'] ?? [ 'default' => null ];
+ $shards = $wgConf->settings['wgCirrusSearchShardCount'] ?? [ 'default' => [] ];

This leads to:

…/tests/cirrusTest.php:248: Undefined index: general
…/tests/cirrusTest.php:248: Undefined index: titlesuggest
…/tests/cirrusTest.php:248: Undefined index: content

So it's clear the test needs something very specific, and does not tolerate a dummy value. Yet, it all happened to work with implicit null, per T232691#5488675.

gerritbot added a comment.Sep 12 2019, 7:45 PM

Change 536329 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[operations/mediawiki-config@master] tests/WgConfTestCase: HACK: Downgrade throw to echo

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

gerritbot added a project: Patch-For-Review.Sep 12 2019, 7:45 PM
gerritbot added a comment.Sep 12 2019, 8:51 PM

Change 536329 merged by jenkins-bot:
[operations/mediawiki-config@master] tests/WgConfTestCase: HACK: Downgrade throw to echo

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

Maintenance_bot removed a project: Patch-For-Review.Sep 12 2019, 9:10 PM
gerritbot added a comment.Sep 12 2019, 9:48 PM

Change 536350 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[operations/mediawiki-config@master] tests: Remove dbconfigTest.php

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

gerritbot added a project: Patch-For-Review.Sep 12 2019, 9:48 PM
gerritbot added a comment.Sep 12 2019, 9:52 PM

Change 536350 merged by jenkins-bot:
[operations/mediawiki-config@master] tests: Remove dbconfigTest.php

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

Maintenance_bot removed a project: Patch-For-Review.Sep 12 2019, 10:10 PM
Jdforrester-WMF added a project: CirrusSearch.Sep 13 2019, 9:38 PM

Tagging back in Cirrus for the remaining issues.

dcausse added a project: Discovery-Search (Current work).Sep 19 2019, 2:16 PM
gerritbot added a comment.Sep 20 2019, 7:33 AM

Change 538156 had a related patch set uploaded (by DCausse; owner: DCausse):
[operations/mediawiki-config@master] [cirrus] fix cirrusTest.php

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

gerritbot added a project: Patch-For-Review.Sep 20 2019, 7:33 AM
Jdforrester-WMF closed this task as Resolved.Sep 20 2019, 8:51 PM
Jdforrester-WMF moved this task from INBOX to Completed on the Release-Engineering-Team-TODO (201909) board.
gerritbot added a comment.Sep 20 2019, 8:52 PM

Change 538156 merged by jenkins-bot:
[operations/mediawiki-config@master] [cirrus] fix cirrusTest.php

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

Maintenance_bot removed a project: Patch-For-Review.Sep 20 2019, 9:10 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits