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

Add PHP 7.2 in CI #489

Merged
merged 4 commits into from
Oct 9, 2017
Merged

Add PHP 7.2 in CI #489

merged 4 commits into from
Oct 9, 2017

Conversation

Jean85
Copy link
Collaborator

@Jean85 Jean85 commented Aug 18, 2017

This is still WIP, it seems that we are getting warning that I would like to investigate further.

@Jean85 Jean85 self-assigned this Aug 18, 2017
@Jean85
Copy link
Collaborator Author

Jean85 commented Aug 18, 2017

There is a session related failure, but there seems to be a few session fixes in 7.2.0beta3, and right now I'm only able to test on beta2 due to Docker Hub availability; I will wait a few days before going forward with this.

@Jean85
Copy link
Collaborator Author

Jean85 commented Aug 23, 2017

It seems that PHPUnit 5 will not allow for a green build under PHP 7.2, due to a deprecated use of each(). This is a really big issue, since it could mean that we would not be able to have a CI without dropping older PHP/PHPUnit versions.

Nevermind, it seems that the deprecation doesn't make the build fail. There is still something that trigger the failure, I'm working on isolating it.

@Jean85
Copy link
Collaborator Author

Jean85 commented Aug 23, 2017

I'm missing something...
test/bootstrap.php does a session_start() that triggers all the issues (but is needed for a few tests), but the failure is triggered only on PHP 7.2; this does seems related to the fact that the headers are already sent so the session id cannot be changed anymore..

But how's that it doesn't bother lower PHP versions?

@nokitakaze
Copy link
Contributor

1) Raven_Tests_ClientTest::testGet_user_data
session_id(): Cannot change session id when headers already sent

/home/travis/build/getsentry/sentry-php/test/Raven/Tests/ClientTest.php:2047

This is just a test. I think you can suppress this warning

@nokitakaze
Copy link
Contributor

Please apply this patch https://pastebin.com/wUf9jgk7

I want to make PR, but it is already created. We need to add PHP 7.2 to supported versions. RC3 is already here http://php.net/archive/2017.php#id2017-09-28-2

We can not wait anymore.

@stayallive
Copy link
Collaborator

@nokitakaze You can create a PR to this branch if you'd like.

Copy link
Contributor

@nokitakaze nokitakaze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need add PHP 7.2 as supported. RC-3 is already here

@Jean85
Copy link
Collaborator Author

Jean85 commented Oct 9, 2017

Thanks @nokitakaze for the help!!

@Jean85 Jean85 merged commit 5958fdf into master Oct 9, 2017
@Jean85 Jean85 changed the title [WIP] Add PHP 7.2 in CI Add PHP 7.2 in CI Oct 9, 2017
@nokitakaze nokitakaze deleted the add_php_7.2 branch October 10, 2017 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants