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

Test failures on PHP 7.4 #920

Closed
osma opened this issue Feb 3, 2020 · 7 comments · Fixed by #1127
Closed

Test failures on PHP 7.4 #920

osma opened this issue Feb 3, 2020 · 7 comments · Fixed by #1127
Assignees
Labels
bug php7 Tag for php7 features tests
Milestone

Comments

@osma
Copy link
Member

osma commented Feb 3, 2020

At which URL did you encounter the problem?

See the Travis build log: https://travis-ci.org/NatLibFi/Skosmos/jobs/645471054

What steps will reproduce the problem?

  1. Try to run Skosmos tests using PHP 7.4

What is the expected output? What do you see instead?

The build fails with warnings like this:

Error: implode(): Passing glue string after array is deprecated. Swap the parameters

There might be other errors as well but this was by far the most prominent.
Found while attempting to upgrade PHP, see #919

@osma osma added bug tests php7 Tag for php7 features labels Feb 3, 2020
@osma osma added this to the Next Tasks milestone Feb 3, 2020
@osma
Copy link
Member Author

osma commented Feb 3, 2020

Not sure if the implode warning is the real issue here. According to the PHP 7.4 deprecated features list, it's deprecated, but in my understanding should just print a warning. In any case, I couldn't find any implode calls with invalid parameter order within the Skosmos code base, though they might be in some library we depend on.

@osma
Copy link
Member Author

osma commented Feb 5, 2020

This EasyRdf PR fixes a number of implode calls. It was merged to EasyRdf master in November 2019 but it's not yet in any released version.

@kouralex
Copy link
Contributor

kouralex commented Jun 3, 2020

There exists a new fork of EasyRdf that aims to address maintenance issues of that project.

What do you think @osma about switching to using it? It would fix this issue straightaway.

@osma
Copy link
Member Author

osma commented Jun 3, 2020

Good catch @kouralex ! The fork looks good but it seems that the final outcome is not yet resolved - it looks possible that the original EasyRdf repo could move to the newly created easyrdf organization account. See this comment in particular. Maybe wait at least a few more days for the dust to settle?

@k00ni
Copy link

k00ni commented Jun 3, 2020

Hi, i am the owner of the EasyRdf fork you mentioned.

It would be helpful if you could post a comment in discussion at easyrdf/easyrdf#320 and tell us what parts of EasyRdf you use and how you think about the whole situation. Thank you in advance.

@osma
Copy link
Member Author

osma commented Feb 18, 2021

Update on current PHP 7.4 status:

@aturbati wrote on skosmos-users:

I completely
removed all traces of php and installed only php 7.4 with all the
required modules and now everything seems to be working fine. I'll do
some more tests and, if everything is ok, I'll upgrade the Skosmos
version on the AGROVOC server as well.

So it seems Skosmos is now working on PHP 7.4!

But Travis CI tests on PHP 7.4 are not working, see e.g. this build log

@osma osma self-assigned this Mar 3, 2021
@osma
Copy link
Member Author

osma commented Mar 3, 2021

We are using PHPUnit 7.5.20 which is obsolete and does not support PHP 7.4:
https://phpunit.de/supported-versions.html
This could be at least part of the problem.

Even PHPUnit 8 is no longer fully supported by upstream (it's in Life Support mode). But if we want to keep supporting PHP 7.2 (which is included by default in Ubuntu 18.04) then it looks like we will have to use PHPUnit 8, because PHPUnit 9 requires PHP 7.3+.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug php7 Tag for php7 features tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants