Found this during an incompatibility scan to PHP 5.6

CommentFileSizeAuthor
#9 2826297.patch805 bytesThomas Cys
#2 2826297.patch804 bytesThomas Cys
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Thomas Cys created an issue. See original summary.

Thomas Cys’s picture

Thomas Cys
Primary language Dutch
CreditAttribution: Thomas Cys as a volunteer commented
FileSize
2826297.patch804 bytes
plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach as a volunteer commented
Status: Active » Reviewed & tested by the community

Thanks!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2826297.patch, failed testing.

Thomas Cys’s picture

Thomas Cys
Primary language Dutch
CreditAttribution: Thomas Cys as a volunteer commented

Am i doing something wrong here because my patch is failing?

The last submitted patch, 2: 2826297.patch, failed testing.

The last submitted patch, 2: 2826297.patch, failed testing.

dwebpoint’s picture

dwebpoint CreditAttribution: dwebpoint as a volunteer commented

@Thomas Cys
In order to pass test could you replace $entity = clone entity
from your patch with
$entity = clone $entity.

Thomas Cys’s picture

Thomas Cys
Primary language Dutch
CreditAttribution: Thomas Cys as a volunteer commented
FileSize
2826297.patch805 bytes

@dwebpoint
I can't believe i missed that, thanks for pointing that out!

Thomas Cys’s picture

Thomas Cys
Primary language Dutch
CreditAttribution: Thomas Cys as a volunteer commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 9: 2826297.patch, failed testing.

stefanos.petrakis@gmail.com’s picture

stefanos.petrakis@gmail.com
Primary language English
Location Biel, Switzerland
CreditAttribution: stefanos.petrakis@gmail.com as a volunteer and commented
Status: Needs work » Reviewed & tested by the community
Related issues: +#2836238: Entity Translation Menu (entity_translation_i18n_menu) compatibility check against the i18n_menu (7.x-1.x-1.14), +#2836239: Update tests for i18n_menu

Can't believe I missed that too. Excellent catch @dwebpoint!

It is however not relevant to the failing of the tests. just ran the tests (using ET-Beta5 and i18n-1.14).
and got the 4 fails you also got from the bot (https://www.drupal.org/pift-ci-job/529800):
createTranslation, testMenuLocalization, editPage, testMenuLocalizationCustomSourceLanguage
without applying your patch.

Running the tests using i18n-1.13 did not cause the fails, with and without applying your patch.

It seems to me that the latest stable i18n is causing the EntityTranslationMenuTranslationTestCase tests to fail.
Note that this is a relatively recent update (22.10.2016) and the EntityTranslationMenuTranslationTestCase may need to be updated. And the entity_translation_i18n_menu module's code even more. But these are separate issues.

Since this patch does not seem to be causing the errors and it's actually quite easy to judge on correctness, I will RTBC this.

  • plach committed b947ea8 on 7.x-1.x authored by Thomas Cys 
    Issue #2826297 by Thomas Cys, dwebpoint, stefanos.petrakis: 'clone' is a...
plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach as a volunteer commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed, thanks!

Status: Fixed » Closed (fixed)

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