User tests: Successful: Unsuccessful:
According to the documentation, a DSN connection string should use semicolons as delimiter and not spaces. It should also not make any difference for the normal PHP implementation. Since we have currently ~80 errors in the test suite when running on HHVM and most of these are related to this DSN string, I'm trying to fix this. Funnily, I don't have access to HHVM, so I'm going to try to fix this by running this through the unittests again and again...
Status | New | ⇒ | Pending |
Labels |
Added:
?
?
|
Category | ⇒ | Unit Tests |
So... with this PR, the errors would be reduced from 80 to 11. Is it worth that?
Milestone |
Added: |
Yes it's worth
can't this go for staging and 3.7.x too?
Well, I tried changing the base branch but the PR would've drug in the entirety of the 4.0 branch. But yes, this should go into staging instead of 4.0 only.
These are exactly the changes I have made in my test branches pending release of hhvm 3.15.2 or 3.16 which will resolve these issues. facebook/hhvm#7399 (comment)
BTW some of these issues were cataloged a while back in #10220 although the connection issue was new in 3.15 with the inclusion of pgsl.so in HHVM as noted in facebook/hhvm#7399 (comment)
@Hackwar According to the documentation spaces or semi-colons should work,
The PDO_PGSQL Data Source Name (DSN) is composed of the following elements, delimited by spaces or semicolons:
Fortunately HHVM is working on a fix. But I do agree this is a valid fix/work around (personally, I think in most cases spaces are the worst choice for delimiters)
I have been hesitant to submit the change for our call to pg_set_error_verbosity()
since I'm unsure of any possible downstream consequences (I assume possibly none, but you know what they say about assuming). I'm also hesitant since this issue is fixed in hhvm 3.15.2. At the same time, at one point I suggested the same change as a possible work around.
looks good on code review.
So with those comments, we could merge this.
The change to our call of pg_set_error_verbosity()
is no longer needed as hhvm 3.15.2 has been released. We do still need the change to the separator for the DSN used in the tests as that patch has not been merged (they requested some additional changes).
The HHVM patch to fix the DSN separators to allow semicolon or space has been merged, the HHVM 3.16.0 STS release will have this, and is on the list of potential cherry picks for HHVM 3.15.3 LTS.
HHVM 3.16.0 STS is expected around 10/24/2016 and will automatically be the version tested on Travis CI since we are testing against HHVM latest.
so all we have to do is wait?
@andrepereiradasilva There is no harm in merging as it's a semantics change to our unit test, but yes it will also be fixed by just waiting.
I would suggest backing out the pg_set_error_verbosity()
change if we do chose to merge, as that portion of the PR is no longer needed as of HHVM 3.15.2.
Depends what we consider a minimum supported HHVM version to be. If it's something older than 3.15.2 it'll be needed.
@mbabker Since hhvm 3.15.0 is the first version with pgsql.so in core I would suggest that would be the earlist LTS version to consider. 3.15.2 is also the first version were tests do not error out on memory.
As noted in #10220 (comment) we can pick up any recent LTS version of HHVM for testing on Travis CI or latest or nightly. we can also test with and without HHVM's php 7 mode (although the HHVM php 7 mode is currently broken due to facebook/hhvm#7198 )
We still need a fix for facebook/hhvm#2060 and to solve the JInstallerAdapterTest failures or determine if the JInstallerAdapterTest failures are an HHVM bug before we can determine a minimum HHVM version to support. I've been pushing HHVM to include the important bug fixes we need in the patches of 3.15 so that hopefully 3.15 can be a valid LTS release to target for a supported version.
Does the CMS actually work on HHVM practically (and by work I mean is the core extensions + functionality useable not just do the unit tests pass)
Who actually runs HHVM outside of Travis to test that?
It's a lot of work being done if no one uses hhvm
if anyone has a full siteground account, I believe that host does HHVM.
I've seen a few comments from people who tried running J3.4 on HHVM but ran into a lot of issues. Some of the past items have been fixed in the CMS or in HHVM.
At the moment the failures in the unit tests suggest to me that it's unlikely for the CMS to actually run on HHVM without issues. But I haven't tried running anything other than unit tests.
Only on their Cloud Hosting plans - not the regular ones
On 18 October 2016 at 15:12, Walt Sorensen notifications@github.com wrote:
if anyone has a full siteground account, I believe that host does HHVM.
I've seen a few comments from people who tried running J3.4 on HHVM but
ran into a lot of issues. Some of the past items have been fixed in the CMS
or in HHVM.At the moment the failures in the unit tests suggest to me that it's
unlikely for the CMS to actually run on HHVM without issues. But I haven't
tried running anything other than unit tests.—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#12336 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8T1pOOAU5aWOpl3IIw--VI8q6tVFks5q1NO3gaJpZM4KQx4U
.
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/
and it is only an option not a default
On 18 October 2016 at 15:32, Brian Teeman brian@teeman.net wrote:
Only on their Cloud Hosting plans - not the regular ones
On 18 October 2016 at 15:12, Walt Sorensen notifications@github.com
wrote:if anyone has a full siteground account, I believe that host does HHVM.
I've seen a few comments from people who tried running J3.4 on HHVM but
ran into a lot of issues. Some of the past items have been fixed in the CMS
or in HHVM.At the moment the failures in the unit tests suggest to me that it's
unlikely for the CMS to actually run on HHVM without issues. But I haven't
tried running anything other than unit tests.—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#12336 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8T1pOOAU5aWOpl3IIw--VI8q6tVFks5q1NO3gaJpZM4KQx4U
.Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/
Best priced options for HHVM that I could find on a quick search was Google cloud at $5/mo or free trial and A2 Hosting at $5/mo
red hat openshift claims to have a free HHVM option in their developer preview, but I couldn't get it to launch into my account.
Heroku says it offers HHVM as a "highly experimental option", but I haven't had time to deal with a platform that relies heavily on command line for setup and activation. I was looking at heroku as an option since heroku has a free option that can be sandboxed for testing.
The question is "are there any users with hhvm"
On 18 October 2016 at 16:45, Walt Sorensen notifications@github.com wrote:
Best priced options for HHVM that I could find on a quick search was Google
cloud at $5/mo or free trial
https://console.cloud.google.com/launcher/details/bitnami-launchpad/hhvm
and A2 Hosting at $5/mo https://www.a2hosting.com/hhvm-hostingred hat openshift claims to have a free HHVM option
https://hub.openshift.com/quickstarts/127-hhvm in their developer
preview, but I couldn't get it to launch into my account.Heroku says it offers HHVM as a "highly experimental option"
https://devcenter.heroku.com/articles/php-support#selecting-a-runtime-hhvm,
but I haven't had time to deal with a platform that relies heavily on
command line for setup and activation. I was looking at heroku as an option
since heroku has a free option that can be sandboxed for testing
https://www.heroku.com/pricing.—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#12336 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8XSbWbwK-xz0BnspQ4F-Tl_dMz_Qks5q1OmzgaJpZM4KQx4U
.
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/
The question is "are there any users with hhvm"
As I said, I've seen a few comments from people who tried running J3.4.6 on HHVM but ran into a lot of issues. I haven't seen anything more recent.
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-10-29 20:48:51 |
Closed_By | ⇒ | wilsonge |
Seems like this indeed "fixes" that error, however throwing a fatal error because of this: facebook/hhvm#7320
Seems as if we would only support the very latest version of HHVM with Postgres. Can we have such a requirement or do we have to introduce a conditional for that function call?