After returning from Austin, I reconfigured rebuilt our OSUOSL testbots in order to improve their testing performance. However, I was unable to get any of the new builds to cleanly pass a D8 HEAD test, despite using the exact same build process as used for the AWS bots which are currently passing.

After investigating, the only difference between the working AWS builds and broken OSU builds is the PHP version. The AWS builds are using PHP 5.4.28-dotdeb, while the OSU builds are using a newer 5.4.29-dotdeb version. I updated one of the AWS builds to use 5.4.29-dotdeb, and it immediately failed the next two consecutive test runs.

Here are a few of the test runs which have thrown errors:
Updated AWS bot: https://qa.drupal.org/pifr/test/798643
Rebuilt OSU Bot: https://qa.drupal.org/pifr/test/798468
Rebuild OSU Bot: https://qa.drupal.org/pifr/test/798448

I'm seeing inconsistent error counts, and it's different tests each time ... so this appears to be an intermittent issue. Most of the errors look something like this:

Drupal\comment\Tests\CommentStatisticsUnitTest::testRead Erroneous data format for unserializing 'Mock_FakeStatement_25aa8a52' /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/comment/tests/src/CommentStatisticsUnitTest.php:62

Drupal\book\Tests\BookManagerTest::testGetBookParents with data set #2 (array(11, 12), array(11, 2, 10, 11), array(3, 10, 11, 12, 0, 0, 0, 0, 0, 0)) Erroneous data format for unserializing 'Mock_Connection_5ac09061' /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/book/tests/src/BookManagerTest.php:75

Drupal\Tests\Core\Entity\ContentEntityDatabaseStorageTest::testGetTableMappingRevisionable with data set #1 (array('test_id', 'test_bundle', NULL)) Erroneous data format for unserializing 'Mock_Connection_64e8cbdd' /var/lib/drupaltestbot/sites/default/files/checkout/core/tests/Drupal/Tests/Core/Entity/ContentEntityDatabaseStorageTest.php:1049 /var/lib/drupaltestbot/sites/default/files/checkout/core/tests/Drupal/Tests/Core/Entity/ContentEntityDatabaseStorageTest.php:452

CommentFileSizeAuthor
#8 7-8-interdiff.txt2.82 KBalexpott
#8 2287139.8.patch62.86 KBalexpott
#7 2287139.7.patch59.95 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jthorson’s picture

jthorson CreditAttribution: jthorson commented
Title: Various test failures after updating to php 5.4.29 » Various serialization test failures after updating to php 5.4.29
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

Doctrine had the same problem, their fix is interesting.

https://github.com/doctrine/doctrine2/pull/1045/files

Fabianx’s picture

Fabianx CreditAttribution: Fabianx commented

This breaks, because serialization format now is:

O - for normal objects
C - for objects implementing Serialize interface

But correct the right way to create a dynamic object without calling the constructor is using a ReflectionClass.

As Drupal relies on PHP 5.4, we can always use:

-            $object = unserialize(sprintf('O:%d:"%s":0:{}', strlen($name), $name));
+           $rc = new \ReflectionClass($name);
+           $object = $rc->newInstanceWithoutConstructor();

However it might be that the majority of that calls come from PHPUnit, symfony and doctrine. At least a quick search for 'unserialize' did not find much in core itself.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

The first example in the OP is Drupal\Core\Database\Driver\fake\FakeStatement, the other three are Drupal\Core\Database\Connection.
But all of them are 

$this->getMockBuilder('Drupal\Core\Database\Connection')
  ->disableOriginalConstructor()
  ->getMock();

That's not magic or special really...

@jthorson, can you try 5.4.29 with the two commits from #2114823: Update PHPUnit to 4.x rolled back? (da85a66 and 81d8d75).

jthorson’s picture

jthorson CreditAttribution: jthorson commented

These were definitely appearing prior to the PHPUnit update.

Tyrael’s picture

Tyrael CreditAttribution: Tyrael commented

there was a behavior change affecting the unserialization of crafted strings, which was used by some libraries (like doctrine and phpunit-mock-objects).

should be fixed (for now) with https://github.com/sebastianbergmann/phpunit-mock-objects/pull/176 but there are no phpunit release containing the fix yet.

you can read more about the reasoning of the original change at
https://bugs.php.net/bug.php?id=67072
http://www.serverphorums.com/read.php?7,927788
http://www.serverphorums.com/read.php?7,946926
http://www.serverphorums.com/read.php?7,959450

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Active » Needs review
FileSize
2287139.7.patch59.95 KB

I've pinned phpunit-mock-objects to the commit that fixes this.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
FileSize
2287139.8.patch62.86 KB
7-8-interdiff.txt2.82 KB

Forget the changes to composer.json and composer.lock

Status: Needs review » Needs work

The last submitted patch, 8: 2287139.8.patch, failed testing.

jthorson’s picture

jthorson CreditAttribution: jthorson commented
Status: Needs work » Needs review

#8 passed the first test run against 5.4.29. Kicking off a second time, just to verify (as the pattern did seem intermittent, even though it was readily repeatable).

Ignore #9, that was due to a manual cancellation when the wrong bot picked up the test.

jthorson’s picture

jthorson CreditAttribution: jthorson commented
Status: Needs review » Reviewed & tested by the community

Two passes in a row. RTBC.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

  • Commit 9601e39 on 8.x by catch: 
    Issue #2287139 by alexpott: Fixed Various serialization test failures...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented

Wondering if we can just bump this up to a straightforward version requirement here: #2375997: Avoid tying Drupal 8's composer.json to specific package commits.