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

Run automated tests against PHP 7.2 to ensure full compatibility #11936

Open
tsteur opened this Issue Aug 4, 2017 · 8 comments

Comments

Projects
None yet
6 participants
@tsteur
Copy link
Member

tsteur commented Aug 4, 2017

In PHP 7.2 some new methods will be deprecated and trigger notices see https://devzone.zend.com/7628/deprecations-php-7-2/

I have noticed eg create_function() is being used and $php_errormsg. Didn't check all deprecated functions. Goal would be to update libs and code needed to not get any notices.

@mattab

This comment has been minimized.

Copy link
Member

mattab commented Sep 18, 2017

See also this issue in Sparklines: #12065

@mattab mattab added this to the 3.2.0 milestone Sep 18, 2017

@mattab mattab added the Bug label Sep 18, 2017

@mattab

This comment has been minimized.

Copy link
Member

mattab commented Sep 18, 2017

  • Probably we would need to enable the CI builds to run on PHP 7.2
@mneudert

This comment has been minimized.

Copy link
Member

mneudert commented Sep 18, 2017

Just some thoughts on the CI

Would if be a possible solution to use a cron trigger for "non-minimum PHP versions" like 7.x?

Using a special repository containing only one branch for each PHP version with a custom .travis.yml could trigger specific builds without completely overloading Travis. Using a special repository avoids messing with branching on the main repository and should still reasonably ensure support of different PHP versions.

@mattab

This comment has been minimized.

Copy link
Member

mattab commented Sep 18, 2017

It's a nice idea, but we found that it's easier to keep all the jobs into one build as otherwise it gets complicated and time consuming to follow all the builds.
Currently we run our builds on:

  • UI tests on PHP 5.5
  • all other tests on PHP 5.6 (both PDO MySql and Mysqli)

I reckon we only need to test the minimum version 5.5 and the maximum version ie. 7.2?
So maybe we could change to:

  • UI tests on PHP 5.5
  • Alltests suite (which runs Mysqli) on PHP 7.2
  • Unit/integration/system (which runs on PDO) on PHP 5.6

or

  • UI tests on PHP 5.5
  • Alltests suite (which runs Mysqli) on PHP 5.5
  • Unit/integration/system (which runs on PDO) on PHP 7.2

@mattab mattab modified the milestones: 3.2.0, 3.1.2 Sep 21, 2017

@mattab mattab modified the milestones: 3.1.2, 3.2.0 Sep 25, 2017

@mattab mattab changed the title Compatibility with PHP 7.2 Run automated tests against PHP 7.2 to ensure full compatibility Sep 25, 2017

@mattab mattab removed this from the 3.3.0 milestone Oct 6, 2017

@Findus23

This comment has been minimized.

Copy link
Member

Findus23 commented Dec 21, 2017

Now that 7.2 has been released for a month, more and more people are trying to run Piwik with it.

I am using it for a few weeks without any apparent issues. (But SearchEngineKeywordsPerformance is broken because of https://github.com/guzzle/guzzle/pull/1686 (fixed in guzzle 6.3))

But someone on the forum got an error message (once) while updating:
https://forum.piwik.org/t/count-parameter-must-be-an-array-or-an-object-that-implements-countable/26658?u=lukas

@sgiehl

This comment has been minimized.

Copy link
Member

sgiehl commented Dec 21, 2017

I am using it for a few weeks without any apparent issues. (But SearchEngineKeywordsPerformance is broken because of guzzle/guzzle#1686 (fixed in guzzle 6.3))

I'll check that and update dependencies for this plugin in the next release.

@Findus23

This comment has been minimized.

Copy link
Member

Findus23 commented Dec 21, 2017

The plugin seems to be working fine, but it's throwing warnings because of changes in PHP7.2

grafik

@J0WI

This comment has been minimized.

Copy link

J0WI commented Jan 11, 2019

Up to now, test are not even run against PHP 7.1.

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