Page MenuHomePhabricator

Fix WikibaseDataModel CI for php 7.4
Closed, ResolvedPublic

Description

The travis CI build for php 7.4 breaks due to phpunit failures.

Apparently, all of the failing checks have to do with serialization / de-serialization:

  1. Wikibase\DataModel\Tests\ReferenceListTest::testGetValueHashIsTheSameForClone
  2. Wikibase\DataModel\Tests\ReferenceListTest::testSerializationStability
  3. Wikibase\DataModel\Tests\ReferenceListTest::testSerializeUnserializeRoundtrip
  4. Wikibase\DataModel\Tests\ReferenceListTest::testUnserializeCreatesNonIdenticalClones
  5. Wikibase\DataModel\Tests\Statement\StatementTest::testSerialize

In particular, assumptions such as:

	$clone = unserialize( serialize( $original ) );

	$this->assertTrue( $original->equals( $clone ) );
	$this->assertSame( $original->getValueHash(), $clone->getValueHash() );

do not seem to be valid any longer, in PHP 7.4

Possible cause: https://github.com/php/php-src/blob/ea1b8788773fe9d5fd517704da332f0725714b8b/UPGRADING#L148 (?)
Possible solution: https://github.com/php/php-src/blob/ea1b8788773fe9d5fd517704da332f0725714b8b/UPGRADING#L332 (?)

Example failures: https://travis-ci.org/wmde/WikibaseDataModel/jobs/641447598

Related Objects

StatusSubtypeAssignedTask
ResolvedJdforrester-WMF
Resolved toan
ResolvedLucas_Werkmeister_WMDE
ResolvedJoe
ResolvedJoe
ResolvedDzahn
Resolvedhashar
ResolvedJdforrester-WMF
ResolvedLadsgroup
ResolvedMoritzMuehlenhoff
Resolvedjijiki
ResolvedMoritzMuehlenhoff
ResolvedTrizek-WMF
ResolvedDzahn
Resolved Gilles
ResolvedDzahn
ResolvedRequestPapaul
Resolvedjijiki
DeclinedNone
ResolvedDzahn
ResolvedDzahn
ResolvedPapaul
Resolved Cmjohnson
ResolvedRequest Cmjohnson
ResolvedRequestPapaul
ResolvedAndrew
ResolvedArielGlenn
ResolvedDzahn
ResolvedLegoktm
ResolvedPapaul
ResolvedDzahn
Declined Gilles
ResolvedVolans
ResolvedDzahn
ResolvedLegoktm
ResolvedPleaseStand
ResolvedJoe
Resolved tstarling
ResolvedArielGlenn
ResolvedJoe
Resolved tstarling
ResolvedJdforrester-WMF
ResolvedJdforrester-WMF
ResolvedLegoktm
ResolvedJdforrester-WMF
ResolvedDaimona
ResolvedDaimona
ResolvedJdforrester-WMF
ResolvedJoe
ResolvedJMeybohm
ResolvedJoe
ResolvedJoe
ResolvedJoe
ResolvedJoe
ResolvedKrinkle
ResolvedJoe
ResolvedClement_Goubert
ResolvedClement_Goubert
ResolvedClement_Goubert
ResolvedMainframe98
ResolvedJoe

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Addshore moved this task from Incoming to Ready to pick up on the Wikidata-Campsite board.
Addshore moved this task from Backlog to Other on the PHP 7.4 support board.

Lots of spam from php_codesniffer too, I guess T243296: Release a new version of WikibaseCodeSniffer has some baring on that

Lots of spam from php_codesniffer too, I guess T243296: Release a new version of WikibaseCodeSniffer has some baring on that

Yup!

@Addshore Hi! As you've listed this task as a good first task, more details in the task description are welcome. Maybe add where and how one can reproduce this error, link to the codebase, some hints/ideas for fixing the issue, etc. It might be easier that way for a newcomer to get started :)

If you think this task may not be too easy for a newcomer, consider removing the good first task tag! And, same goes for T243588

Is this the same set of issues that emit errors in the CI run?

00:18:23.529 There were 3 errors:
00:18:23.529 
00:18:23.529 1) Wikibase\Client\Tests\Api\ApiPropsEntityUsageTest::testEntityUsage with data set "by title" (array('query', 'entityusage', 'Vienna11|Berlin22'), array(array(array(array(array('O', 'S')))), array(array(array(array('S')), array(array('S'))))))
00:18:23.529 === Logs generated by test case
00:18:23.529 [localisation] [debug] LocalisationCache using store LCStoreNull []
00:18:23.529 [localisation] [debug] LocalisationCache using store LCStoreNull []
00:18:23.529 [MessageCache] [debug] MessageCache using store {class} {"class":"HashBagOStuff"}
00:18:23.529 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
00:18:23.529 ===
00:18:23.529 Trying to access array offset on value of type null
00:18:23.529 
00:18:23.529 /workspace/src/includes/api/ApiPageSet.php:127
00:18:23.529 /workspace/src/extensions/Wikibase/client/tests/phpunit/includes/Api/ApiPropsEntityUsageTest.php:117
00:18:23.529 /workspace/src/extensions/Wikibase/client/tests/phpunit/includes/Api/ApiPropsEntityUsageTest.php:164
00:18:23.529 /workspace/src/extensions/Wikibase/client/tests/phpunit/includes/Api/ApiPropsEntityUsageTest.php:226
00:18:23.529 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:416
00:18:23.529 /workspace/src/maintenance/doMaintenance.php:99
00:18:23.529 
00:18:23.529 2) Wikibase\Client\Tests\Api\ApiPropsEntityUsageTest::testEntityUsage with data set "by entity" (array('query', 'entityusage', 'Vienna11|Berlin22', 'Q3|Q4'), array(array(array(array(array('O', 'S')))), array(array(array(array('S')), array(array('S'))))))
00:18:23.529 === Logs generated by test case
00:18:23.529 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
00:18:23.529 [localisation] [debug] LocalisationCache using store LCStoreNull []
00:18:23.529 [localisation] [debug] LocalisationCache using store LCStoreNull []
00:18:23.529 [MessageCache] [debug] MessageCache using store {class} {"class":"HashBagOStuff"}
00:18:23.529 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
00:18:23.529 ===
00:18:23.529 Trying to access array offset on value of type null
00:18:23.529 
00:18:23.529 /workspace/src/includes/api/ApiPageSet.php:127
00:18:23.529 /workspace/src/extensions/Wikibase/client/tests/phpunit/includes/Api/ApiPropsEntityUsageTest.php:117
00:18:23.529 /workspace/src/extensions/Wikibase/client/tests/phpunit/includes/Api/ApiPropsEntityUsageTest.php:164
00:18:23.529 /workspace/src/extensions/Wikibase/client/tests/phpunit/includes/Api/ApiPropsEntityUsageTest.php:226
00:18:23.529 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:416
00:18:23.529 /workspace/src/maintenance/doMaintenance.php:99
00:18:23.529 
00:18:23.529 3) Wikibase\Client\Tests\Changes\ChangeRunCoalescerTest::testCoalesceChanges with data set "local link breaks" (array(Wikibase\Lib\Changes\ItemChange Object (...), Wikibase\Lib\Changes\ItemChange Object (...)), array(Wikibase\Lib\Changes\ItemChange Object (...), Wikibase\Lib\Changes\ItemChange Object (...)))
00:18:23.529 === Logs generated by test case
00:18:23.529 [objectcache] [debug] MainWANObjectCache using store {class} {"class":"EmptyBagOStuff"}
00:18:23.529 ===
00:18:23.529 Wikibase\Client\Changes\ChangeRunCoalescer::coalesceRuns:array_key_exists(): Using array_key_exists() on objects is deprecated. Use isset() or property_exists() instead [Called from Wikibase\Client\Changes\ChangeRunCoalescer::coalesceRuns in /workspace/src/extensions/Wikibase/client/includes/Changes/ChangeRunCoalescer.php at line 287]
00:18:23.529 
00:18:23.529 /workspace/src/includes/debug/MWDebug.php:333
00:18:23.529 /workspace/src/includes/debug/MWDebug.php:188
00:18:23.529 /workspace/src/includes/GlobalFunctions.php:1068
00:18:23.529 /workspace/src/extensions/Wikibase/client/includes/Changes/ChangeRunCoalescer.php:287
00:18:23.529 /workspace/src/extensions/Wikibase/client/includes/Changes/ChangeRunCoalescer.php:80
00:18:23.529 /workspace/src/extensions/Wikibase/client/tests/phpunit/includes/Changes/ChangeRunCoalescerTest.php:386
00:18:23.529 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:416
00:18:23.529 /workspace/src/maintenance/doMaintenance.php:99

Is this the same set of issues that emit errors in the CI run?

Apparently, no. Filed T247595 for that.

PHP 7.4 is stable, and now in a version of Ubuntu (20.04) that is likely to pick up usage...

Does this make it worth bumping up the priority a bit?

Addshore raised the priority of this task from Lowest to Medium.May 20 2020, 6:51 PM

Will make the followup described in the PR now

kostajh subscribed.

This seems to be an issue again now that CI is running PHP 7.4. See https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-php74-noselenium-docker/56135/console

09:46:58 There was 1 failure:
09:46:58 
09:46:58 1) Wikibase\DataModel\Tests\ReferenceListTest::testSerializationStability
09:46:58 Failed asserting that two strings are identical.
09:46:58 --- Expected
09:46:58 +++ Actual
09:46:58 @@ @@
09:46:58 -'a:1:{i:0;O:28:"Wikibase\DataModel\Reference":1:{s:35:"Wikibase\DataModel\Referencesnaks";O:32:"Wikibase\DataModel\Snak\SnakList":2:{s:4:"data";a:1:{i:0;C:43:"Wikibase\DataModel\Snak\PropertyNoValueSnak":2:{P1}}s:5:"index";i:0;}}}'
09:46:58 +'a:1:{i:0;O:28:"Wikibase\DataModel\Reference":1:{s:35:"Wikibase\DataModel\Referencesnaks";C:32:"Wikibase\DataModel\Snak\SnakList":100:{a:2:{s:4:"data";a:1:{i:0;C:43:"Wikibase\DataModel\Snak\PropertyNoValueSnak":2:{P1}}s:5:"index";i:0;}}}}'
09:46:58 
09:46:58 /workspace/src/extensions/Wikibase/lib/packages/wikibase/data-model/tests/unit/ReferenceListTest.php:545
09:46:58 
09:46:58 FAILURES!
09:46:58 Tests: 24800, Assertions: 86154, Failures: 1, Skipped: 97.

The Wikibase\DataModel\Tests\ReferenceListTest::testSerializationStability is:

/**
 * This test will change when the serialization format changes.
 * If it is being changed intentionally, the test should be updated.
 * It is just here to catch unintentional changes.
 */
public function testSerializationStability() {
        $list = new ReferenceList();
        $list->addNewReference( new PropertyNoValueSnak( 1 ) );

        /*
         * https://wiki.php.net/rfc/custom_object_serialization
         */
        if ( version_compare( phpversion(), '7.4', '>=' ) ) {
                $testString = "a:1:{i:0;O:28:\"Wikibase\\DataModel\\Reference\":1:{s:35:\"\x00Wikibase\\DataModel\\"
                        . "Reference\x00snaks\";O:32:\"Wikibase\\DataModel\\Snak\\SnakList\":2:{s:4:\""
                        . 'data";a:1:{i:0;C:43:"Wikibase\\DataModel\\Snak\\PropertyNoValueSnak":2:{P1}}s:5'
                        . ':"index";i:0;}}}';
        } else {
                $testString = "a:1:{i:0;O:28:\"Wikibase\\DataModel\\Reference\":1:{s:35:\"\x00Wikibase\\DataModel\\"
                        . "Reference\x00snaks\";C:32:\"Wikibase\\DataModel\\Snak\\SnakList\":100:{a:2:{s:4:\""
                        . 'data";a:1:{i:0;C:43:"Wikibase\\DataModel\\Snak\\PropertyNoValueSnak":2:{P1}}s:5'
                        . ':"index";i:0;}}}}';
        }

        $this->assertSame(
                $testString,
                $list->serialize()
        );
}

It does take in account serialization has changed with php 7.4 and the code is correct. The test fails on our CI because we have patched php7.4 to use the old serialization format, the switch got made as part of upgrading Quibble to 1.4.6 https://gerrit.wikimedia.org/r/c/integration/config/+/828611

The reason we had to patch php 7.4 is the Wikimedia cluster currently runs a mix of php 7.2 and 7.4. Thus when a process runs under php 7.4, the serialized object is stored in the cache with the new format. When a later process running with php 7.2 attempts to unserialize the new format, it fails leading to T316601.

The approach we took was to patch php 7.4 to use the old format as described at T316601#8201209. This way we can finish the migration on the Wikimedia cluster. For this failing test, it means that even if it varies its expectations based on phpversion() >= 7.4, when it runs on our patched php 7.4 the serialization is done with the old format. The test break.

@Jakob_WMDE and @Silvan_WMDE raised it on IRC in #wikimedia-releng. We concluded the easiest course of action right now is to mark the test skipped and restore it once Wikimedia has fully migrated to php 7.4 and that we move the CI images to the original php 7.4 version.