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

Travis: run the build tests against PHP 7.2 as well #511

Merged
merged 2 commits into from
Oct 16, 2017

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Sep 13, 2017

The new Trusty images as per Sept 7 include an image for PHP 7.2 (even though it hasn't been released yet) and nightly is now PHP 7.3-dev.

@jrfnl jrfnl requested a review from wimg September 13, 2017 23:20
@wimg
Copy link
Member

wimg commented Sep 13, 2017

Looks like it's failing on 2 builds ?

@jrfnl
Copy link
Member Author

jrfnl commented Sep 14, 2017

Two on the PR build, three on the push build. Hmm... Those failures don't have anything to do with the change in this PR, but might well be related to more changes in the Travis PHP images.

The new Trusty images as per Sept 7 include an image for PHP 7.2 (even though it hasn't been released yet) and nightly is now PHP 7.3-dev.

Also, while the Travis images in Q2 required PHP 5.4 to be run on `precise` for the builds to pass, seems that in Q3, `trusty` is fixed again.
@jrfnl jrfnl force-pushed the feature/travis-build-against-php-7.2 branch from b40a280 to 7932def Compare September 14, 2017 00:14
@jrfnl
Copy link
Member Author

jrfnl commented Sep 14, 2017

I've changed the PHP 5.4 builds back to using trusty for those two builds - they needed precise in Q2. Finger crossed this will fix it.

@jrfnl
Copy link
Member Author

jrfnl commented Oct 11, 2017

Right. Finally took the time to try & figure out what is going on here.

I've traced the errors back to the PHPUnit version being used for those builds. Specifically to a change made in PHPUnit 4.6.0 which is when the builds start breaking in combination with PHP 5.3 and 5.4 and older PHPCS versions.
Running PHPCS manually with the specific version combinations over the test files which fail in the unit test run, yield the correct results, so it's not PHPCS failing, but something in the BaseSniffTest class in combination with certain PHPUnit versions.

The relevant current Travis images use:

PHP PHPUnit PHPCS 1.x PHPCS 2.x PHPCS 3.x
5.3.29 4.8.18 (was 4.5.0) N/A
5.4.45 (was 5.4.37) 4.8.35 (was 4.5.0)

Strangely enough the build for PHP 5.4 / PHPCS 3.x, both for the old as well as for the new images, seems to have been run on 5.4.45/4.8.35 and passes without issue (confirming that it is something in the BaseSniffTest class).

Sadly, I've not been able to track down which specific change in PHPUnit is the one causing the problem and have therefore not been able to figure out a way to mitigate this. I suspect it has something to do with the command line options being passed to PHPCS and PHPUnit getting confused over it, but debugging this has so far not yielded any results.

What I'd like to propose is to just fix the PHPUnit version for the 3 affected builds to PHPUnit 4.5.1 and be done with it. At least that way the builds will pass again and new PRs can be pulled.

Refs:

@jrfnl jrfnl force-pushed the feature/travis-build-against-php-7.2 branch from e21cdcc to 70b2795 Compare October 11, 2017 05:13
@wimg
Copy link
Member

wimg commented Oct 11, 2017

Given that HHVM is no longer supported by any of the big frameworks, should we keep support for it at all ?

@jrfnl
Copy link
Member Author

jrfnl commented Oct 11, 2017

Given that HHVM is no longer supported by any of the big frameworks, should we keep support for it at all ?

I'm ambivalent about it. Basically as long as we don't need to do any extra coding in the sniffs for the builds to pass, I'm happy enough to leave it be. It can help devs who dev on HHVM, but deploy to PHP.
Having said that, as I'm not one of them, I'm easily swayed to drop it 😉

@wimg wimg merged commit 861801a into master Oct 16, 2017
@wimg wimg deleted the feature/travis-build-against-php-7.2 branch October 16, 2017 15:04
@jrfnl
Copy link
Member Author

jrfnl commented Oct 25, 2017

@wimg What should the milestone for this one be ? 8.0.2 or 8.1.0 ?

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

Successfully merging this pull request may close these issues.

2 participants