Last modified: 2014-11-10 18:27:25 UTC
The unit tests in mediawiki-core-regression-hhvm-master fail (cf. https://integration.wikimedia.org/ci/job/mediawiki-core-regression-hhvm-master/2479/console): | [...] | 18:40:04 There were 10 failures: | 18:40:04 | 18:40:04 1) FormatJsonTest::testParseTryFixing with data set #3 ('[1],', false) | 18:40:04 Failed asserting that true is false. | 18:40:04 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/includes/json/FormatJsonTest.php:199 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/MediaWikiTestCase.php:141 | 18:40:04 | 18:40:04 2) FormatJsonTest::testParseTryFixing with data set #4 ('[1,]', '[1]') | 18:40:04 Failed asserting that true is false. | 18:40:04 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/includes/json/FormatJsonTest.php:201 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/MediaWikiTestCase.php:141 | 18:40:04 | 18:40:04 3) FormatJsonTest::testParseTryFixing with data set #5 ('[1 | 18:40:04 ,]', '[1]') | 18:40:04 Failed asserting that true is false. | 18:40:04 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/includes/json/FormatJsonTest.php:201 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/MediaWikiTestCase.php:141 | 18:40:04 | 18:40:04 4) FormatJsonTest::testParseTryFixing with data set #6 ('[1, | 18:40:04 ]', '[1]') | 18:40:04 Failed asserting that true is false. | 18:40:04 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/includes/json/FormatJsonTest.php:201 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/MediaWikiTestCase.php:141 | 18:40:04 | 18:40:04 5) FormatJsonTest::testParseTryFixing with data set #7 ('[1,] | 18:40:04 ', '[1]') | 18:40:04 Failed asserting that true is false. | 18:40:04 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/includes/json/FormatJsonTest.php:201 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/MediaWikiTestCase.php:141 | 18:40:04 | 18:40:04 6) FormatJsonTest::testParseTryFixing with data set #8 ('[1 | 18:40:04 , | 18:40:04 ] | 18:40:04 ', '[1]') | 18:40:04 Failed asserting that true is false. | 18:40:04 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/includes/json/FormatJsonTest.php:201 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/MediaWikiTestCase.php:141 | 18:40:04 | 18:40:04 7) FormatJsonTest::testParseTryFixing with data set #9 ('["a,",]', '["a,"]') | 18:40:04 Failed asserting that true is false. | 18:40:04 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/includes/json/FormatJsonTest.php:201 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/MediaWikiTestCase.php:141 | 18:40:04 | 18:40:04 8) FormatJsonTest::testParseTryFixing with data set #10 ('[[1,] | 18:40:04 ,[2, | 18:40:04 ],[3 | 18:40:04 ,]]', '[[1],[2],[3]]') | 18:40:04 Failed asserting that true is false. | 18:40:04 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/includes/json/FormatJsonTest.php:201 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/MediaWikiTestCase.php:141 | 18:40:04 | 18:40:04 9) FormatJsonTest::testParseTryFixing with data set #11 ('[[1,],[2,],[3,]]', false) | 18:40:04 Failed asserting that true is false. | 18:40:04 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/includes/json/FormatJsonTest.php:199 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/MediaWikiTestCase.php:141 | 18:40:04 | 18:40:04 10) FormatJsonTest::testParseTryFixing with data set #12 ('[1,,]', false) | 18:40:04 Failed asserting that true is false. | 18:40:04 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/includes/json/FormatJsonTest.php:199 | 18:40:04 /mnt/jenkins-workspace/workspace/mediawiki-core-regression-hhvm-master/src/tests/phpunit/MediaWikiTestCase.php:141 | 18:40:04 | [...] (I am confused by the Jenkins UI at the moment. At https://integration.wikimedia.org/ci/job/mediawiki-core-regression-hhvm-master/, I'm not sure if "Last successful build" refers to the /chronological/ order in which the tests are run, or if that reflects the Git ancestry (that being useful, the former not).)
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.
Change 170251 had a related patch set uploaded by BryanDavis: hhvm: fix FormatJsonTest::testParseTryFixing for lenient json parser https://gerrit.wikimedia.org/r/170251
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: | [...] | 2) FormatJsonTest::testParseTryFixing with data set #3 ('[1],', false, true, '[1]') | Expected isGood() == true | Failed asserting that false matches expected true. | /home/travis/build/wikimedia/mediawiki/tests/phpunit/includes/json/FormatJsonTest.php:236 | /home/travis/build/wikimedia/mediawiki/tests/phpunit/MediaWikiTestCase.php:141 | 3) FormatJsonTest::testParseTryFixing with data set #4 ('[1,]', '[1]') | Expected isGood() == true | Failed asserting that false matches expected true. | /home/travis/build/wikimedia/mediawiki/tests/phpunit/includes/json/FormatJsonTest.php:236 | /home/travis/build/wikimedia/mediawiki/tests/phpunit/MediaWikiTestCase.php:141 | 4) FormatJsonTest::testParseTryFixing with data set #5 ('[1 | ,]', '[1]') | Expected isGood() == true | Failed asserting that false matches expected true. | /home/travis/build/wikimedia/mediawiki/tests/phpunit/includes/json/FormatJsonTest.php:236 | /home/travis/build/wikimedia/mediawiki/tests/phpunit/MediaWikiTestCase.php:141 | 5) FormatJsonTest::testParseTryFixing with data set #6 ('[1, | ]', '[1]') | Expected isGood() == true | Failed asserting that false matches expected true. | /home/travis/build/wikimedia/mediawiki/tests/phpunit/includes/json/FormatJsonTest.php:236 | /home/travis/build/wikimedia/mediawiki/tests/phpunit/MediaWikiTestCase.php:141 | 6) FormatJsonTest::testParseTryFixing with data set #7 ('[1,] | ', '[1]') | Expected isGood() == true | Failed asserting that false matches expected true. | /home/travis/build/wikimedia/mediawiki/tests/phpunit/includes/json/FormatJsonTest.php:236 | /home/travis/build/wikimedia/mediawiki/tests/phpunit/MediaWikiTestCase.php:141 | 7) FormatJsonTest::testParseTryFixing with data set #8 ('[1 | , | ] | ', '[1]') | Expected isGood() == true | Failed asserting that false matches expected true. | /home/travis/build/wikimedia/mediawiki/tests/phpunit/includes/json/FormatJsonTest.php:236 | /home/travis/build/wikimedia/mediawiki/tests/phpunit/MediaWikiTestCase.php:141 | 8) FormatJsonTest::testParseTryFixing with data set #9 ('["a,",]', '["a,"]') | Expected isGood() == true | Failed asserting that false matches expected true. | /home/travis/build/wikimedia/mediawiki/tests/phpunit/includes/json/FormatJsonTest.php:236 | /home/travis/build/wikimedia/mediawiki/tests/phpunit/MediaWikiTestCase.php:141 | 9) FormatJsonTest::testParseTryFixing with data set #10 ('[[1,] | ,[2, | ],[3 | ,]]', '[[1],[2],[3]]') | Expected isGood() == true | Failed asserting that false matches expected true. | /home/travis/build/wikimedia/mediawiki/tests/phpunit/includes/json/FormatJsonTest.php:236 | /home/travis/build/wikimedia/mediawiki/tests/phpunit/MediaWikiTestCase.php:141 | 10) FormatJsonTest::testParseTryFixing with data set #11 ('[[1,],[2,],[3,]]', false, true, '[[1],[2],[3]]') | Expected isGood() == true | Failed asserting that false matches expected true. | /home/travis/build/wikimedia/mediawiki/tests/phpunit/includes/json/FormatJsonTest.php:236 | /home/travis/build/wikimedia/mediawiki/tests/phpunit/MediaWikiTestCase.php:141 | 11) FormatJsonTest::testParseTryFixing with data set #12 ('[1,,]', false, false, '[1]') | Expected isOK == true | Failed asserting that false is true. | /home/travis/build/wikimedia/mediawiki/tests/phpunit/includes/json/FormatJsonTest.php:237 | /home/travis/build/wikimedia/mediawiki/tests/phpunit/MediaWikiTestCase.php:141 | [...] 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.
Change 172090 had a related patch set uploaded by BryanDavis: hhvm: Detect json-c parser https://gerrit.wikimedia.org/r/172090
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
