Problem/Motivation

Started having test failures like this:

efault value for disabled_container_datetime: expected array ( 'date' => '1978-11-01 10:30:00', 'timezone_type' => 3, 'timezone' => 'Europe/Berlin', ), returned array ( 'date' => '1978-11-01 10:30:00.000000', 'timezone_type' => 3, 'timezone' => 'Europe/Berlin', ).

Tracked it down to this change in PHP: https://github.com/php/php-src/commit/fdb2709dd27c5987c2d2c8aaf0cdbebf9f...

We currently hardcode the expected value.

Proposed resolution

Use json_decode(json_encode()) so that the expected matches whatever a given PHP version will output.

Not great, but what we are doing is a bit... special so I think that's ok. Could think of more problematic issues with that change, though.

Remaining tasks

We could consider checking with PHP to undo this or something, but versions with it are already out, so I think it's not likely that will happen and the broken PHP versions are out there.

User interface changes

API changes

CommentFileSizeAuthor
#8 2316153-fix-typo-8.patch1.16 KBBerdir
#1 json_encode-2316153-1.patch1.18 KBBerdir
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Active » Needs review
FileSize
json_encode-2316153-1.patch1.18 KB
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Version: 8.x-dev » 8.0.x-dev
chx’s picture

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

Last time I tried to talk to the PHP team about BC I was told new warnings are not breaking backwards compatibility. Good luck convincing them that this one does.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Just confirmed this with PHP master/NG/5.7, that I prepared for performance*/compatibility tests. FormTest HEAD fails, passes with the patch.

* First result on that was this test. 5.4: 1m37s. 5.7: 57s. Promising start.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

That's a very silly workaround for us to have to do, but I don't have any other bright ideas. :\

We're very near an alpha, but this is holding up provisioning of new testbots, and there are multiple sprints coming up this week. So...

Committed and pushed to 8.x. Thanks!

  • webchick committed eda0bc5 on 8.0.x 
    Issue #2316153 by Berdir: Fixed PHP 5.4.30/5.5.15+ changed json_encode...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented 
+++ b/core/modules/system/tests/modules/form_test/src/Form/FormTestDisabledElementsForm.php
@@ -128,7 +128,11 @@ public function buildForm(array $form, FormStateInterface $form_state) {
+    // microsends. Make sure that the expected value is correct for all versions

microsends.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Title: PHP 5.4.30/5.5.15+ changed json_encode() of DateTime objects to include microseconds, breaks tests » [typo follow-up] PHP 5.4.30/5.5.15+ changed json_encode() of DateTime objects to include microseconds, breaks tests
Status: Fixed » Needs review
FileSize
2316153-fix-typo-8.patch1.16 KB

Ups.

chx’s picture

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

hap shittens, it's OK.

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!

  • catch committed 9af55ec on 8.0.x 
    Issue #2316153 by Berdir: Fixed [typo follow-up] PHP 5.4.30/5.5.15+...

Status: Fixed » Closed (fixed)

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

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented
Version: 8.0.x-dev » 8.1.x-dev

Let's port this to 8.1.x-dev as well as it breaks there (ref: https://www.drupal.org/node/2384853#comment-9391473)

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Version: 8.1.x-dev » 8.0.x-dev

As discussed on IRC.

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented

Thanks @Berdir. @alexpott said that 8.1.x is not open for development and we got the branch just to postpone issues on it. So ignore #13