WordPress.org

Make WordPress Core

Opened 5 months ago

Closed 3 months ago

#26153 closed defect (bug) (worksforme)

test_json_encode_decode fails in PHP 5.4

Reported by: georgestephanis Owned by:
Milestone: Priority: low
Severity: minor Version: 3.8
Component: External Libraries Keywords: has-patch
Focuses: Cc:

Description

test_json_encode_decode fails in PHP 5.4 because the PEAR JSON compat class doesn't declare isError as a static function on line 864.

http://core.trac.wordpress.org/browser/trunk/src/wp-includes/class-json.php#L864

This really won't ever cause any actual problems because it's only there for compat with PHP 5.2, and static doesn't kick problems until 5.4 -- so it's just when running Unit Tests that the class is included and explicitly run that it can whine.

Patch attempts to fix this, by just testing for both the functions json_encode and json_decode existing, and that they work properly.

On 5.2, they will include the class automatically, and test cleanly. On 5.4, it'll just test the native functions. This feels better to me than testing a class that won't ever be included, or modifying an imported class to add a static keyword.

Attachments (1)

test_json_encode_decode.diff (923 bytes) - added by georgestephanis 5 months ago.

Download all attachments as: .zip

Change History (9)

comment:2 nacin5 months ago

This test is "to verify Services_JSON is intact and working". The patch no longer tests Services_JSON.

comment:3 georgestephanis5 months ago

And in 5.2, it would test that, because it would be included and called by json_encode() and json_decode().

If we mean to test the library in a version of php that I don't believe it would ever need to be included on, would you be okay with updating it by adding a static keyword so that it runs cleanly on 5.4?

comment:4 SergeyBiryukov5 months ago

FWIW, I could not reproduce the issue on PHP 5.4.6. The test runs successfully for me, and I didn't disable E_STRICT warnings or anything like that.

On 5.2, they will include the class automatically, and test cleanly. On 5.4, it'll just test the native functions.

PHP 5.2 has native json_encode() and json_decode() too, so the version doesn't matter here. What matters is whether PHP was compiled with the JSON extension (see #18015).

comment:5 georgestephanis5 months ago

It kicked up for me on a sandbox running 5.4.21

comment:6 SergeyBiryukov5 months ago

Could not reproduce with 5.4.22 either.

However, when I removed error suppression from Tests_POMO_PO::test_prepend_each_line() (added in [25395]), I was able to receive the warning for that test. Weird.

comment:7 SergeyBiryukov5 months ago

So, I can only reproduce if I call Services_JSON::isError() in a test manually:

Non-static method Services_JSON::isError() should not be called statically, assuming $this from incompatible context

For the instances inside the class itself, I don't see the warning. Wouldn't it be the same as this::isError(), which works for non-static methods too?

comment:8 nacin3 months ago

  • Component changed from Unit Tests to External Libraries
  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.