HHVM has a lenient json parser which allows trailing commas. The failing tests seem to be ones where the test expects json_decode() to fail with JSON_ERROR_SYNTAX and then need to have extra commas stripped.

Block that ignores trailing commas: https://github.com/facebook/hhvm/blob/2d64184dd67068e02d3069bbc045bca031498465/hphp/runtime/ext/json/JSON_parser.cpp#L707-L720

$ /usr/bin/php5 -r 'var_dump( json_decode("[0,]") );'
NULL

$ /usr/bin/hhvm --php -r 'var_dump( json_decode("[0,]") );'
array(1) {

[0]=>
int(0)

}

I'm really not sure why it is always enabled. It looks like there is an option flag to enable/disable the behavior.

gerritadmin wrote:

Change 170251 had a related patch set uploaded by BryanDavis:
hhvm: fix FormatJsonTest::testParseTryFixing for lenient json parser

https://gerrit.wikimedia.org/r/170251

gerritadmin wrote:

Change 170251 merged by jenkins-bot:
hhvm: fix FormatJsonTest::testParseTryFixing for lenient json parser

https://gerrit.wikimedia.org/r/170251

Passing now. https://integration.wikimedia.org/ci/job/mediawiki-core-regression-hhvm-master/2495/console

00:05:49.197 OK, but incomplete or skipped tests!
00:05:49.201 Tests: 9141, Assertions: 1600131, Skipped: 18.

Interestingly, while this has silenced the Jenkins tests, the change has introduced failures in the same spots with Travis CI's HHVM: The commit before passed (cf. https://travis-ci.org/wikimedia/mediawiki/jobs/39621924), this one (cf. https://travis-ci.org/wikimedia/mediawiki/jobs/39624363) and the ones after fail now with:

Panta rhei.

(In reply to Tim Landscheidt from comment #6)

Interestingly, while this has silenced the Jenkins tests, the change has
introduced failures in the same spots with Travis CI's HHVM: The commit
before passed (cf. https://travis-ci.org/wikimedia/mediawiki/jobs/39621924),
this one (cf. https://travis-ci.org/wikimedia/mediawiki/jobs/39624363) and
the ones after fail now with:

[...]

2) FormatJsonTest::testParseTryFixing with data set #3 ('[1],', false,

true, '[1]')

Expected isGood() == true

Failed asserting that false matches expected true.

I wonder if the HHVM packages that the WMF is building have some Facebook default settings enabled that aren't enabled in the builds used by Travis CI? When I looked through the upstream source for HHVM it looked to me like the JSON loose parsing behavior should not have been enabled by default and instead would require a custom HHVM option (JSON_FB_LOOSE) to be passed to json_decode to enable the loose parse behavior.

I'll look a bit deeper and see if I can figure this out. I think that if we are enabling this in our build it is probably by accident. If nothing else, I can probably craft a guard to add to the test that detects if the default behavior is loose or not.

Ori pointed out that the CMake rules will use a shared libjson-c library if present and the WMF HHVM build is using that:

$ ldd /usr/bin/hhvm |grep json

libjson-c.so.2 => /lib/x86_64-linux-gnu/libjson-c.so.2 (0x00007ff80354e000)

I haven't investigated yet if this would change json_decode() to be lenient by default, but it is a possibility.

(In reply to Bryan Davis from comment #8)

Ori pointed out that the CMake rules will use a shared libjson-c library if
present and the WMF HHVM build is using that:

$ ldd /usr/bin/hhvm |grep json

libjson-c.so.2 => /lib/x86_64-linux-gnu/libjson-c.so.2

(0x00007ff80354e000)

I haven't investigated yet if this would change json_decode() to be lenient
by default, but it is a possibility.

Libjson-c is lenient by default https://github.com/json-c/json-c/blob/d4e81f9ec8273914739808737fa0a27a3f0589fb/json_tokener.h#L90-L100 and this is not changed by HHVM https://github.com/facebook/hhvm/blob/a82234b63e86d57026db719e533225f25f103127/hphp/runtime/ext/json/jsonc_parser.cpp#L225-L227. This combination means that the behavior of json_decode() changes depending on how HHVM is compiled/linked.

gerritadmin wrote:

Change 172090 had a related patch set uploaded by BryanDavis:
hhvm: Detect json-c parser

https://gerrit.wikimedia.org/r/172090

gerritadmin wrote:

Change 172090 merged by jenkins-bot:
hhvm: Detect json-c parser

https://gerrit.wikimedia.org/r/172090

Passing in https://travis-ci.org/wikimedia/mediawiki/jobs/40564023