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

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

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.

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).

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)

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

• Looks like we have another issue in the hhvm pgsql.so with our driver Fatal error: Call to unimplemented native function pg_set_client_encoding() in /joomla-cms/libraries/joomla/database/driver/postgresql.php on line 890 the pg_set_client_encoding() function is not implemented in HHVM for various reasons
  • this was solved in 3.6.1 staging

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.

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.

@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?

I believe #11565 fixes the memcache issues in HHVM.

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 (=_=)

sorry to ear that :(

shall we remove the hhvm tests on travis them?

(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

yes, i agree

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.

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.

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.

nice

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"

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.

Yes, that is correct @mbabker less memory use by the test suit would complete and report (less than ~255mb) there was a bug in HHVM related to failing to Save ini values so they could be restored with ini_restore() which was the root cause of memory_get_peak_usage(true) failing if the memory use was greater than 256mb and preventing the report. So solved thanks to less tests and with the old test suite in staging when hhvm 3.15.2 or 3.16 is released. Here is a build I've ran showing it's working in hhvm-nightly (3.16) https://travis-ci.org/photodude/joomla-cms/jobs/164806859 (trying to see what's going on with PDO pgsql)

The connection issue with Postgresql

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

is due to how HHVM was parsing the DSN for pdo_pgsql.so it was expecting semicolons as delimiters not spaces. An issue is open with HHVM and we should have a fix hopefully in hhvm 3.15.2

We can also fix it on our end by modifying the getConnection() to use semi-colons rather than spaces /tests/unit/core/case/database/postgresql.php but we'll still have to wait until the release of hhvm 3.15.2 for the fix to undefined function pg_set_error_verbosity() example build

@photodude can you have a look here: #12341 & #12336

The biggest HHVM testing issue to solve at the moment is the JInstallerAdapterTest failure(s).
My best guess on those failures are due to something with the SQLite in HHVM preventing the insertion of columns for the #__extensions table. so far I have not been able to find away to trace the steps leading up to the failure.

For the JDatabaseDriverPostgresqlTest::testReleaseTransactionSavepoint
pg_result_error_field() expects parameter 1 to be resource, boolean given error caused by this call to pg_result_error_field() when $this->cursor is false from a query failing.

Seems like we need to better handle this error situation

maybe something like (adjusted to show Exception)

    protected function getErrorNumber()
    {
        if ($this->cursor === false)
        {
            $this->errorMsg = pg_last_error($this->connection);

            throw new JDatabaseExceptionExecuting($this->sql, $this->errorMsg);
        }

        return (int) pg_result_error_field($this->cursor, PGSQL_DIAG_SQLSTATE) . ' ';
    }

Is this a reasonable course to solve the issue?

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.

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

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

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.

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

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

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).

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

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).

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

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?

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

@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.

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
.

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) )

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)

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

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.

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

#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.

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

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

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

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

PR was merged. Nice work @photodude

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

1. At 77% of unit tests it takes a long time (something is strange here)
2. There is a "Invalid configuration directive" at 68%. see https://travis-ci.org/joomla/joomla-cms/jobs/183001506#L1275

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

@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

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

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