Automattic / babble

Multilingual WordPress done right.
https://wordpress.org/plugins/babble/
245 stars 50 forks source link

Failing Behat build on PHP 5.6 #285

Closed simonwheatley closed 9 years ago

simonwheatley commented 9 years ago

See: ccc9eed631769671403940a49d54f5df47533ea5

The Travis build was failing for PHP 5.6, possibly related to the PHP server command in bin/script.sh. We should fix this build failure so we can test in PHP 5.6. :)

johnbillion commented 9 years ago

The recent builds are passing on PHP 5.6: https://travis-ci.org/Automattic/babble/builds/69915111

Seeing as these build tests pass on other plugins of mine which use the same code, it might have been an intermittent issue on Travis.

I'm going to remove the allowed failures for 5.6 and see how it goes.

johnbillion commented 9 years ago

Scrub that, those builds don't branch from the new Behat branches.

johnbillion commented 9 years ago

Still failing on 5.6: https://travis-ci.org/Automattic/babble/builds/70880928

simonwheatley commented 9 years ago

@johnbillion What do you reckon to closing this issue? Recently these builds have been passing.

joehoyle commented 9 years ago

Well, after several hours of nasty behat debugging, I tracked this down to a 3 level deep dependancy in Goutte. The issue is that the cookies as not being sent with the requests, so things like WordPress login does not work. I opened a issue upstream here: https://github.com/FriendsOfPHP/Goutte/issues/232 however, I'm not sure how long that is going to take to propagate in updates up the dependancy chain we have.

It appears fabpot/goutte an use any of 3 versions, which might explain why this used to pass under PHP 5.4. When doing a fresh install, the Behat tests also fail under 5.5.9.

I have a hack fix (mentioned on the above ticket) but I'm not sure the best way to apply that, or if that's even a good idea.

Another work around would be to run the web server from port 80, rather than 8080, then we won't run into this edgecase. I'm not sure if we have the Travis tests running as sudo though, which one would need to do that. (which also means no containerization).

I'm going to open a PR to change to port 80.

joehoyle commented 9 years ago

Should be fixed by #321

joehoyle commented 9 years ago

We are now passing on 5.6 all around :) closing out!