Problem/Motivation

The 7.x branch currently has test failures on PHP 5.3: https://www.drupal.org/pift-ci-job/1234762

Proposed resolution

We can resolve by setting test assertion message (thus avoiding calls to t() with invalid UTF-8).

CommentFileSizeAuthor
#18 interdiff.txt1.19 KBjordanwood
#18 3047844-ignore-warning-18.patch1.44 KBjordanwood
#5 3047844-ignore-warning-5.patch1.04 KBmfb
#4 3047844-ignore-warning-4.patch676 bytesmfb
#3 3047844-ignore-warning-3.patch589 bytesmfb
#2 3047844-ignore-warning.patch710 bytesmfb
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mfb created an issue. See original summary.

mfb’s picture

mfb
they or he
CreditAttribution: mfb commented
Status: Active » Needs review
FileSize
3047844-ignore-warning.patch710 bytes
mfb’s picture

mfb
they or he
CreditAttribution: mfb commented
FileSize
3047844-ignore-warning-3.patch589 bytes
mfb’s picture

mfb
they or he
CreditAttribution: mfb commented
FileSize
3047844-ignore-warning-4.patch676 bytes
mfb’s picture

mfb
they or he
CreditAttribution: mfb commented
FileSize
3047844-ignore-warning-5.patch1.04 KB
mfb’s picture

mfb
they or he
CreditAttribution: mfb commented
Issue summary: View changes
joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Drupal 7.66 target

looks good thanks

mfb’s picture

mfb
they or he
CreditAttribution: mfb commented
Issue tags: -Drupal 7.66 target +Drupal 7.67 target
mcdruid’s picture

mcdruid
Location 🇬🇧🇪🇺
CreditAttribution: mcdruid commented
Issue tags: -Drupal 7.67 target +Drupal 7.68 target
joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented

Easy win here.

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Related issues: +#3088836: Rejuvenate D7, a need for more time spent committing and RTBCing Drupal 7 issues
joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Priority: Normal » Major
Related issues: -#3088836: Rejuvenate D7, a need for more time spent committing and RTBCing Drupal 7 issues

Existing test coverage is sufficient.

Bumping the priority up (this is very important), lots of sites are still on php 5.3.x , I recently worked at an engineering school in Montréal that had about 120 Drupal sites using php 5.3.x. They have quite an elaborate setup using aegir and multisite with impressive functionality.

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented
Issue tags: +Pending Drupal 7 commit

Triage, this is a high priority issue so that we can re-queue any patches against php 5.3.x before committing to core.

apaderno’s picture

apaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
CreditAttribution: apaderno as a volunteer commented
Title: D7 test failures on PHP 5.3 » Tests fail on PHP 5.3
Taran2L’s picture

Taran2L
Primary language Ukrainian
Location Lviv
CreditAttribution: Taran2L at Lemberg Solutions commented

Ha, I've found this during my Drupal 7 support PHP7.4 run, and the patch is pretty much the same (except for the wording).

For the reference, patch from #3081386-22: [META] Fully support PHP 7.4 in Drupal 7 and the wording:

  • The invalid filename has been detected and RuntimeException has been thrown
  • The invalid destination has been detected and RuntimeException has been thrown

+1 to RTBC

Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Novice

Purely putting to CNW do that a Novice sees this as actionable. (Else would have left RTBC)

We really like the strings from the PHP 7.4 issue much better.

Novice instructions

- Change descriptions per above comment
- Add new patch
- Set back to RTBC
- Remove Novice tag

Thank you!

jordanwood’s picture

jordanwood CreditAttribution: jordanwood at Northern Commerce commented
Assigned: Unassigned » jordanwood

I'll work on this

jordanwood’s picture

jordanwood CreditAttribution: jordanwood at Northern Commerce commented
Assigned: jordanwood » Unassigned
Status: Needs work » Needs review
FileSize
3047844-ignore-warning-18.patch1.44 KB
interdiff.txt1.19 KB

Here is a new patch with new error messages.

jordanwood’s picture

jordanwood CreditAttribution: jordanwood at Northern Commerce commented
Issue tags: -Novice
Fabianx’s picture

Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commented
Title: Tests fail on PHP 5.3 » [Regression] Tests fail on PHP 5.3
Assigned: Unassigned » mcdruid
Priority: Major » Critical
Status: Needs review » Reviewed & tested by the community

Back to RTBC

RTBM from me and over to mcdruid for commit.

  • mcdruid committed aeef877 on 7.x 
    Issue #3047844 by mfb, jordanwood, Taran2L: Fix test failures on PHP 5.3
mcdruid’s picture

mcdruid
Location 🇬🇧🇪🇺
CreditAttribution: mcdruid at Acquia commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Pending Drupal 7 commit

Thank you everyone that contributed!

joseph.olstad’s picture

joseph.olstad
Primary language French
CreditAttribution: joseph.olstad commented

Thanks @mcdruid, and congrats on your first real core commit other than the MAINTAINERS.txt !

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-4
CreditAttribution: mparker17 at Northern Commerce commented

Many firsts: this was @jordanwood's first accepted patch to any project as well! Congratulations to both of you! 🙂

Status: Fixed » Closed (fixed)

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