Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HHVM errors from Travis ci #10220

Closed
photodude opened this issue May 4, 2016 · 54 comments
Closed

HHVM errors from Travis ci #10220

photodude opened this issue May 4, 2016 · 54 comments

Comments

@photodude
Copy link
Contributor

photodude commented May 4, 2016

This is a list of the tests which are known to be failing or have errors in the HHVM phpunit testing (allowed failures)

Unfortunately, due to an out memory limit error in Travis-ci I can only retrieve the names of the tests that errored or failed (test listed below)

I was able to get some of the test failure details by filtering and only running one test at a time.

you can see the results of my first test here https://travis-ci.org/photodude/joomla-cms/jobs/127702652

There were 11 9 0 errors:

1) JInstallerAdapterTest::testCheckExistingExtensionForExistingExtension
UnexpectedValueException: No columns found for #__extensions table
/libraries/joomla/table/table.php:245
/libraries/joomla/table/table.php:162
/libraries/joomla/table/extension.php:30
/tests/unit/suites/libraries/cms/installer/JInstallerAdapterTest.php:103
2) JInstallerAdapterTest::testCheckExistingExtensionForExtensionThatDoesNotExist
UnexpectedValueException: No columns found for #__extensions table
/libraries/joomla/table/table.php:245
/libraries/joomla/table/table.php:162
/libraries/joomla/table/extension.php:30
/tests/unit/suites/libraries/cms/installer/JInstallerAdapterTest.php:148
3) JInstallerAdapterTest::testDiscoverInstall
UnexpectedValueException: No columns found for #__extensions table
/libraries/joomla/table/table.php:245
/libraries/joomla/table/table.php:162
/libraries/joomla/table/extension.php:30
/tests/unit/suites/libraries/cms/installer/JInstallerAdapterTest.php:371
4) JInstallerAdapterTest::testDiscoverInstallWithNoDescription
UnexpectedValueException: No columns found for #__extensions table
/libraries/joomla/table/table.php:245
/libraries/joomla/table/table.php:162
/libraries/joomla/table/extension.php:30
/tests/unit/suites/libraries/cms/installer/JInstallerAdapterTest.php:439
5) JInstallerAdapterTest::testInstall
UnexpectedValueException: No columns found for #__extensions table
/libraries/joomla/table/table.php:245
/libraries/joomla/table/table.php:162
/libraries/joomla/table/extension.php:30
/tests/unit/suites/libraries/cms/installer/JInstallerAdapterTest.php:827
6) JInstallerAdapterTest::testInstallOnUpdateRoute
UnexpectedValueException: No columns found for #__extensions table
/libraries/joomla/table/table.php:245
/libraries/joomla/table/table.php:162
/libraries/joomla/table/extension.php:30
/tests/unit/suites/libraries/cms/installer/JInstallerAdapterTest.php:901
7) JInstallerAdapterTest::testInstallAbortsWhenSetupUpdatesThrowsException
UnexpectedValueException: No columns found for #__extensions table
/libraries/joomla/table/table.php:245
/libraries/joomla/table/table.php:162
/libraries/joomla/table/extension.php:30
/tests/unit/suites/libraries/cms/installer/JInstallerAdapterTest.php:980
8) JInstallerAdapterTest::testInstallWithNoDescription
UnexpectedValueException: No columns found for #__extensions table
/libraries/joomla/table/table.php:245
/libraries/joomla/table/table.php:162
/libraries/joomla/table/extension.php:30
/tests/unit/suites/libraries/cms/installer/JInstallerAdapterTest.php:1052
9) JInstallerAdapterTest::testParseQueriesWithUpdateRouteAndParsingReturningTrueCallsParseSchemaUpdatesCorrectly
UnexpectedValueException: No columns found for #__extensions table
/libraries/joomla/table/table.php:245
/libraries/joomla/table/table.php:162
/libraries/joomla/table/extension.php:30
/tests/unit/suites/libraries/cms/installer/JInstallerAdapterTest.php:1330

See proposed PR #12990

10) JDocumentHtmlTest::testEnsureMergeHeadDataReturnsThisObject
Array to string conversion
/tests/unit/core/helper/helper.php:52
/libraries/joomla/document/html.php:222
/tests/unit/suites/libraries/joomla/document/html/JDocumentHTMLTest.php:134

See merged PR #12013

11) JDatabaseDriverPostgresqlTest::testReleaseTransactionSavepoint
pg_result_error_field() expects parameter 1 to be resource, boolean given
/home/travis/build/photodude/joomla-cms/tests/unit/core/helper/helper.php:52
/home/travis/build/photodude/joomla-cms/libraries/joomla/database/driver/postgresql.php:1513
/home/travis/build/photodude/joomla-cms/libraries/joomla/database/driver/postgresql.php:742
/home/travis/build/photodude/joomla-cms/libraries/joomla/database/driver/postgresql.php:1384
/home/travis/build/photodude/joomla-cms/tests/unit/suites/database/driver/postgresql/JDatabaseDriverPostgresqlTest.php:1205

See merged PR #12359

There were 3 2 failures

Fatal error: Call to undefined function pg_set_error_verbosity() in /home/travis/build/photodude/joomla-cms/libraries/joomla/database/driver/postgresql.php on line 140 fixed in the release of HHVM 3.15.2

1) JInstallerAdapterTest::testParseQueriesWithUpdateRouteAndParsingReturningFalseReturnsException
Expectation failed for method name is equal to string:parseSchemaUpdates when invoked 1 time(s).
Method was expected to be called 1 times, actually called 0 times.
See proposed PR #12990

2) JLoaderTest::testSetupWithoutPrefixes
Failed asserting that true is false.
/tests/unit/suites/libraries/joomla/JLoaderTest.php:621

See facebook/hhvm#2060 for related issue to JLoaderTest
See PR #12478 for a merged internal solution

3) JUserHelperTest::testGetCryptedPassword
Password is hashed to crypt-blowfish without salt
Failed asserting that false is true.
/tests/unit/suites/libraries/joomla/user/JUserHelperTest.php:443

See merged PR #12428 for a salted fix

At the conclusion of testing memory fails Fatal error: request has exceeded memory limit in /libraries/vendor/phpunit/php-timer/src/Timer.php on line 97 Fixed in hhvm 3.16.0 and 3.15.2

Invalid configuration directive notice
'JLanguageTest::testParse' This gives a notice but no failure or error

@photodude
Copy link
Contributor Author

When the HHVM php7 compatibility flag is on, the errors greatly increase. Currently tests die at about 12% Click here to see an example build...

This is the error that seems to be killing the HHVM_with_PHP7_compatibility flag unitTests

Methods with the same name as their class will not be constructors in a future version of PHP; Cache_Lite has a deprecated constructor: 1x
    1x in JCacheStorageTest::casesGetInstance

@mbabker
Copy link
Contributor

mbabker commented May 20, 2016

That should only be a deprecation notice and not a failure. That or HHVM has a behavior change compared to PHP 7.

@photodude
Copy link
Contributor Author

photodude commented May 20, 2016

According to HHVM when the flag hhvm.php7.all = 1 is set to fully enable PHP 7 mode one of the things it does is "Disallow and warn when using old PHP 4 constructors"

Seems HHVM with php7 mode may be far more strict about some of the PHP7 changes.

There is a PR for Cache_Lite that will resolve the PHP7 deprecated constructor issue.

@photodude
Copy link
Contributor Author

photodude commented May 20, 2016

At some point a decision about what HHVM LTS versions we want to test against will be needed. There is a feature request and a PR for Travis CI that will allow Trusty available HHVM LTS versions If/when that PR gets merged we will have the following options 3.3, 3.6, 3.9, 3.12, and when it's released in July hhvm 3.15.

Currently, we only have the option to test against hhvm latest (currently 3.13 which is partially broken and has no ability to do PostgreSQL), or hhvm-nightly (currently 3.14 dev) and with/without PHP7 mode (php7 mode for hhvm is only available in 3.11 or newer).

@photodude
Copy link
Contributor Author

photodude commented May 25, 2016

Allow HHVM LTS version specification #733 has been implemented and documented.

we now have access to the following

  • hhvm-3.3
  • hhvm-3.6
  • hhvm-3.9
  • hhvm-3.12
  • hhvm-3.15
  • hhvm
  • hhvm-nightly

note: hhvm is latest (not always an LTS version), and hhvm-nightly is current nightly dev of HHVM

I suggest not testing against LTS versions older than 3.12since that is the first LTS with an option for php7 mode

That would suggest a maximum HHVM build matrix option as follows

    - php: hhvm-3.15
      sudo: true
      dist: trusty
      group: edge # until the next update
      addons:
        apt:
          packages:
            - mysql-server-5.6
            - mysql-client-core-5.6
            - mysql-client-5.6
      services:
        - mysql
        - postgresql
        - memcache
        - memcached
        - redis-server
      env: HHVMPHP7="no" INSTALL_APCU="no" INSTALL_APCU_BC_BETA="no" # Disabled items that currently do not work in travis-ci hhvm
    - php: hhvm-3.15
      sudo: true
      dist: trusty
      group: edge # until the next update
      addons:
        apt:
          packages:
            - mysql-server-5.6
            - mysql-client-core-5.6
            - mysql-client-5.6
      services:
        - mysql
        - postgresql
        - memcache
        - memcached
        - redis-server
      env: HHVMPHP7="yes" INSTALL_APCU="no" INSTALL_APCU_BC_BETA="no" # Disabled items that currently do not work in travis-ci hhvm
    - php: hhvm
      sudo: true
      dist: trusty
      group: edge # until the next update
      addons:
        apt:
          packages:
            - mysql-server-5.6
            - mysql-client-core-5.6
            - mysql-client-5.6
      services:
        - mysql
        - postgresql
        - memcache
        - memcached
        - redis-server
      env: HHVMPHP7="no" INSTALL_APCU="no" INSTALL_APCU_BC_BETA="no" # Disabled items that currently do not work in travis-ci hhvm
    - php: hhvm
      sudo: true
      dist: trusty
      group: edge # until the next update
      addons:
        apt:
          packages:
            - mysql-server-5.6
            - mysql-client-core-5.6
            - mysql-client-5.6
      services:
        - mysql
        - postgresql
        - memcache
        - memcached
        - redis-server
      env: HHVMPHP7="yes" INSTALL_APCU="no" INSTALL_APCU_BC_BETA="no" # Disabled items that currently do not work in travis-ci hhvm
    - php: hhvm-nightly
      sudo: true
      dist: trusty
      group: edge # until the next update
      addons:
        apt:
          packages:
            - mysql-server-5.6
            - mysql-client-core-5.6
            - mysql-client-5.6
      services:
        - mysql
        - postgresql
        - memcache
        - memcached
        - redis-server
      env: HHVMPHP7="no" INSTALL_APCU="no" INSTALL_APCU_BC_BETA="no" # Disabled items that currently do not work in travis-ci hhvm
    - php: hhvm-nightly
      sudo: true
      dist: trusty
      group: edge # until the next update
      addons:
        apt:
          packages:
            - mysql-server-5.6
            - mysql-client-core-5.6
            - mysql-client-5.6
      services:
        - mysql
        - postgresql
        - memcache
        - memcached
        - redis-server
      env: HHVMPHP7="yes" INSTALL_APCU="no" INSTALL_APCU_BC_BETA="no" # Disabled items that currently do not work in travis-ci hhvm
  allow_failures:
    - php: hhvm-3.15
    - php: hhvm
    - php: hhvm-nightly
before_script:
  - composer install
  - if [[ $HHVMPHP7 == "yes" ]]; then echo hhvm.php7.all=1 >> /etc/hhvm/php.ini; fi

This travis build matrix will build 6 versions of HHVM

  • hhvm-3.15
  • hhvm-3.15 (with PHP7 mode)
  • hhvm
  • hhvm (with PHP7 mode)
  • hhvm-nightly
  • hhvm-nightly (with PHP7 mode)

@photodude
Copy link
Contributor Author

photodude commented May 25, 2016

HHVM postgresql driver issue the pg_set_error_verbosity() function is not implemented in HHVM for various reasons

This missing function causes Fatal error: Call to undefined function pg_set_error_verbosity() in joomla-cms/libraries/joomla/database/driver/postgresql.php on line 140

See test build on hhvm-3.12 with hhvm pgsql.so

@photodude
Copy link
Contributor Author

photodude commented May 26, 2016

@photodude
Copy link
Contributor Author

photodude commented Jul 19, 2016

In reviewing the JInstallerAdapterTest failures I've come to the conclusion that the JInstallerAdapterTest failures are due to something with the SQLite implimentation in HHVM. I believe the HHVM installed modules are pdo_sqlite and SQLite3 I don't know if that makes any difference.

@photodude
Copy link
Contributor Author

photodude commented Jul 20, 2016

JDocumentHtmlTest::testEnsureMergeHeadDataReturnsThisObject Array to string conversion failure is due to an intentional difference in the HHVM array_unique() implimentation

HHVM's array_unique() method works by converting all the items in the arrays to strings then comparing, so an array to string notice is thrown. Zend does the conversion to string each time it does a compare.

@photodude
Copy link
Contributor Author

@mbabker In checking with HHVM on the JDocumentHtmlTest::testEnsureMergeHeadDataReturnsThisObject Array failure with HHVM's array_unique() method; they indicated the this is related to PHPUnit converting notices to exceptions.

I tried looking into what might need to be done with our PHPUnit tests to deal with the PHP errors so that the test will pass, but I'm more lost now than before on how to solve this testing issue.

I also think that the PHPUnit converting notices to exceptions is also related to the php cache lite test failure on the notice Disallow and warn when using old PHP 4 constructors issue with HHVM in PHP7 mode.

Do you have any thoughts on how to deal with this?

@photodude
Copy link
Contributor Author

I believe #11565 fixes the memcache issues in HHVM.

@photodude
Copy link
Contributor Author

I'm not sure we are going to be able to get PHP unit and HHVM to work together.

The owner/maintainer of PHPunit doesn't care about HHVM anymore and is likely to drop HHVM from their travis builds

Maybe this is a dead road in the project now (=_=)

@andrepereiradasilva
Copy link
Contributor

andrepereiradasilva commented Aug 28, 2016

sorry to ear that :(

shall we remove the hhvm tests on travis them?

@photodude photodude reopened this Aug 28, 2016
@photodude
Copy link
Contributor Author

photodude commented Aug 28, 2016

(Stupid phone, closing things because of small buttons and big fingers)

@andrepereiradasilva I'm trying to see if I can find out if PHP Unit is going to drop the possibility of HHVM for sure, or if they are just currently annoyed with the past state of the project. If PHP unit is definitely dropping support for HHVM then we need to figure out if we are going to find a different unit test option for HHVM, or if we are going to kill HHVM support development.

IMO That's more of a PLT call

@andrepereiradasilva
Copy link
Contributor

yes, i agree

@photodude
Copy link
Contributor Author

I got access to the "alpha way to initiate a debug job via an API call on travis" and tried to figure out what was going on with the memory issue. After a few tests I had to "hack" the /libraries/vendor/phpunit/php-timer/src/Timer.php to work around the failure caused by memory_get_peak_usage(true) at the end of the tests.

I have edited the list of tests that error or fail to reflect the existing issues that would need to be addressed for the unit tests to pass.

@photodude
Copy link
Contributor Author

I got acknowledgment from HHVM of the bug that was causing our "Fatal error: request has exceeded memory limit" issue. They have implemented a patch that should fix the issue in the next release (I assume 3.15.1)

I'm still working with HHVM to get a fix to the HHVM-nightly packages so I can test the patch and verify that the memory issue caused by memory_get_peak_usage(true) in the phpunit timer is actually solved.

@photodude
Copy link
Contributor Author

So the bug in HHVM causing the "Fatal error: request has exceeded memory limit" issue has been fixed in HHVM 3.16.0-dev (hhvm-nightly) unfortunately the patch got missed in the 3.15.1 LTS release. hopefully it gets included in the 3.15.2 LTS release.

@andrepereiradasilva
Copy link
Contributor

nice

@photodude
Copy link
Contributor Author

photodude commented Oct 3, 2016

HHVM testing note with the complete test run:
Time: 2.92 minutes, Memory: 270.03MB
Tests: 6119, Assertions: 10341, Errors: 80, Failures: 3, Skipped: 119, Incomplete: 40.
User deprecation notices (60)

A number of the Errors are new and due to a newly introduced connection issue with Postgresql

"PDOException: [112]: could not translate host name "localhost port=5432 dbname=joomla_ut" to address: Name or service not known"

@mbabker
Copy link
Contributor

mbabker commented Oct 7, 2016

The memory issue is in part the size of the test suite, in part our own configurations and code structure. If you look at the last HHVM builds for staging and 4.0-dev (where about half the test suite has been dumped thanks to dropping code for the Framework stack and relying on its tests) the HHVM build runs in full on both but the memory runs out at the end of the cycle on staging but no such issue with 4.0.

@photodude
Copy link
Contributor Author

photodude commented Oct 8, 2016

Maybe not a full solution.... the underlying error for the failure of JDatabaseDriverPostgresqlTest::testReleaseTransactionSavepoint is ERROR: no such savepoint which does not have an error code that could be parsed easily...

At least we know a little more.

@photodude
Copy link
Contributor Author

Maybe adjust it to throw new JDatabaseExceptionExecuting($this->sql, $this->errorMsg); when $this->cursor is false from a query failing.

@photodude
Copy link
Contributor Author

Here is the PR for the suggested change to the Postgresql driver #12359

@Hackwar
Copy link
Member

Hackwar commented Oct 10, 2016

Hey @photodude thanks for all your work on this. I've looked into this, too, and you already saw one of my fixes. The array_unique() issue is unfortunate, to say the least. I think the behavior of HHVM here is wrong, but anyway. Regarding the JDatabaseDriverPostgresqlTest::testReleaseTransactionSavepoint error, this seems like a bigger issue, since we don't seem to be using the Postgres functions correctly...

I still think that keeping compatible to HHVM would be good.

@csthomas
Copy link
Contributor

  1. JDocumentHtmlTest::testEnsureMergeHeadDataReturnsThisObject
    Array to string conversion
    /tests/unit/core/helper/helper.php:52
    /libraries/joomla/document/html.php:222
    /tests/unit/suites/libraries/joomla/document/html/JDocumentHTMLTest.php:134

#10220 (comment)

is fixed in PR #12013

@photodude
Copy link
Contributor Author

@csthomas That's awesome glad to see the fix, I hope it's merged soon.

@photodude
Copy link
Contributor Author

photodude commented Oct 15, 2016

The following error will not be fixed in HHVM as noted in this issue facebook/hhvm#1071

3) JUserHelperTest::testGetCryptedPassword
Password is hashed to crypt-blowfish without salt
Failed asserting that false is true.
/tests/unit/suites/libraries/joomla/user/JUserHelperTest.php:443

This is returning *0 indicating a failure (guessing the DES in not valid or there is a bug in HHVM).

@photodude
Copy link
Contributor Author

JUserHelperTest::testGetCryptedPassword failure is fixed by correcting our internal method, HHVM is telling us we didn't form the salt correctly. See PR #12428

@photodude
Copy link
Contributor Author

I additionally suggest that the JUserHelperTest::testGetCryptedPassword failure can be skipped in the HHVM testing since the GetCryptedPassword() method was replaced in 3.2.1 and is only there for BC considerations. Since 3.2.0 will never be HHVM compatible and 3.2.1+ doesn't use the method, we can effectively ignore this issue with 3.6.3+ and 4.0 doesn't have this method (any 3rd party extensions still using the old method should have changed).

@photodude
Copy link
Contributor Author

I believe this HHVM issue facebook/hhvm#2060 is the cause of the failure in our JLoaderTest::testSetupWithoutPrefixes test on HHVM

@photodude
Copy link
Contributor Author

Looks like we have PR's or have discovered HHVM bugs for all of the test issues and errors with the singular exception of the JInstallerAdapterTest items.

Anyone have suggestions on how to proceed with solving those?

@photodude
Copy link
Contributor Author

photodude commented Oct 18, 2016

I made a small change to one of the JInstallerAdapterTest tests from $this->getMockDatabase() to an already stored call to this object (array) $mockDatabase the change works in php but I got a new error in HHVM

Fatal error: Call to undefined method
PHPUnit_Framework_MockObject_InvocationMocker::getTableColumns() in
/libraries/joomla/table/table.php on line 241

@photodude
Copy link
Contributor Author

photodude commented Oct 19, 2016

@mbabker I don't know much about the _autoloader in JLoader, can _autoloader be set to public? The references I've looked at have just identified _autoloader as function without public/private static options. or can we switch to use the spl_autoload_register() function?

Again sorry for my lack of experience and knowledge on autoloaders.

@mbabker
Copy link
Contributor

mbabker commented Oct 19, 2016

I honestly have no clue why it was marked private, but ever since the auto
loader has been there, people have been filing issues because it is
incompatible with third-party code that uses public functions.

On Tuesday, October 18, 2016, Walt Sorensen notifications@github.com
wrote:

@mbabker https://github.com/mbabker I don't know much about the
_autoloader in JLoader
https://github.com/joomla/joomla-cms/blob/staging/libraries/loader.php#L578-L647,
can _autoloader and _load be set to public? The references I've looked at
http://php.net/manual/en/function.autoload.php have just identified
those as function without public/private static options. or can we switch
to use the spl_autoload_register() function?

Again sorry for my lack of experience and knowledge on autoloaders.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#10220 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoVAsWWjdzW2oaoO6k5mypS7b5p2bks5q1Y_7gaJpZM4IW1pu
.

@photodude
Copy link
Contributor Author

Thanks @mbabker my only assumption is it got marked private by habit since in php 4 there was a preference to mark methods private or protected with leading underscores.

In this instance, it seems like marking this private is the completely wrong thing to do. (it's questionable why it even works in php facebook/hhvm#959 (comment) )

@photodude
Copy link
Contributor Author

photodude commented Oct 20, 2016

We now have solutions in proposed PRs, Merged PRs, or HHVM bug fixes for all known issues except the JInstallerAdapterTest items and the JLanguageTest::testParse invalid configuration notice .

Once everything is merged, and the few errors remaining are fixed, it should be reasonable to get people attempting to live test on HHVM again (something I don't think people have done since J3.3 and J3.4 due to the errors we had to fix and the bugs in HHVM prior to HHVM 3.15.2)

@photodude
Copy link
Contributor Author

Biggest block to live testing with HHVM is now just the failures with the JInstallerAdapterTest

@photodude
Copy link
Contributor Author

The more I try to dig into the JInstallerAdapterTest failures the more this is looking like an issue with PHPUnit 4.x and HHVM.

My best guess is there is an issue with the mocking of JTableExtension in the JInstallerAdapter tests.

I'm at a bit of a loss on how to solve that issue. Best case scenario is this is only a phpunit/hhvm issue and the J3.7.0 nightly build for staging should be good to go for real world testing on HHVM 3.15.2 with mysql or hhvm 3.16.0-dev nightly with PostgreSQL.

@photodude
Copy link
Contributor Author

photodude commented Nov 26, 2016

The fix for the JInstallerAdapter failure on hhvm is included in PR #12990

There are some new PDOMySql failures in the HHVM tests but that is due to a Travis configuration issue. I'm still waiting for Travis to fix that error since it's due to some change they did in the Trusty testing images.

Once we merge PR #12990 and either PR #12428 for a salted crypt fix or #12668 for a PR that just skips this old crypt test since it's not used in core and has been deprecated. Then we can close this issue and IMO we can include HHVM as a testing option for the 3.7 alpha release

@mbabker
Copy link
Contributor

mbabker commented Nov 26, 2016

#12668 for a PR that just skips this old crypt test since it's not used in core and has been deprecated

Not acceptable. Whether an API is used in core or not, deprecated or not, it should still function as expected until the day it's removed. Likewise, if the class has tests, those should continue to function to validate the behavior.

@photodude
Copy link
Contributor Author

Ok I'll close #12668 as not a valid solution to the testing issue.

@photodude
Copy link
Contributor Author

Ok so to close this issue we need to merge PR #12990 and PR #12428

@brianteeman
Copy link
Contributor

@photodude if everything here is finally resolved with PR #12990 and PR #12428 then it can be closed here?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/10220.

@photodude
Copy link
Contributor Author

@brianteeman Just waiting for PR #12990 to reach RTC or merged. With that PR everything is finally resolved with the HHVM tests.

@andrepereiradasilva
Copy link
Contributor

andrepereiradasilva commented Dec 11, 2016

PR was merged. Nice work @photodude

Still i notice two things tht can be improved in hhvm tests:

But maybe this issue should be closed a new one open for that issues.

@photodude
Copy link
Contributor Author

@andrepereiradasilva, I'm not sure about the slow tests, there seems to be about 6 tests that are significantly slower than the others.

As for the "invalid configuration directive" this seems to be just a notice from hhvm. There is an issue open on it with hhvm

@andrepereiradasilva
Copy link
Contributor

ok @photodude close this. i will open a new issue for the hhvm slow tests

@photodude
Copy link
Contributor Author

Closing as the HHVM tests are now passing with all the PRs having been merged.

@andrepereiradasilva this is the open issue with hhvm on the "invalid configuration directive" notice facebook/hhvm#7279

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants