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

Fix test suite to be run in PHP5 and PHP7 env #58

Merged
merged 1 commit into from
May 22, 2017

Conversation

kipit
Copy link
Contributor

@kipit kipit commented May 18, 2017

Add a compatibility layer to be able to run the test suite in PHP5 or PHP7 env. This will fix Travis builds.

@kipit
Copy link
Contributor Author

kipit commented May 18, 2017

Well, I thought the build would be a success… But HHVM fails. From what I can see, in the build log, the problem is at the infrastructure level, no? Can we improve this situation from our code? Or is it on the Travis-CI team side?

@kipit
Copy link
Contributor Author

kipit commented May 18, 2017

Well… the hhvm build is successful, but now the php-5.3 one is broken :(

@kipit kipit force-pushed the fix_test_suite branch from 5b0f294 to c47bcc1 Compare May 18, 2017 00:39
@tsmes
Copy link
Contributor

tsmes commented May 18, 2017

Just a suggestion: Looking at PHPs overview of eol versions, perhaps the tests should drop everything < 5.6 as that is the oldest still supported?

http://php.net/eol.php

PHP 5.3 entered eol almost 3 years ago,

@willdurand
Copy link
Owner

Yes we can drop EOL'ed versions.

@kipit
Copy link
Contributor Author

kipit commented May 18, 2017

It's my day, I guess: PHP-7.0 build is now the only broken… But I don't know what we can do to make it green.

@kipit kipit force-pushed the fix_test_suite branch from d330626 to 792384e Compare May 19, 2017 20:50
@kipit kipit mentioned this pull request May 19, 2017
@@ -2,7 +2,17 @@

namespace EmailReplyParser\Tests;

class TestCase extends \PHPUnit_Framework_TestCase
if (class_exists('PHPUnit_Framework_TestCase')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if both classes exist at the same, we should use the namespaced one IMO

Copy link
Owner

Choose a reason for hiding this comment

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

Agreed. I was quickly trying something first.

composer.json Outdated
@@ -23,5 +23,8 @@
"branch-alias": {
"dev-master": "2.5-dev"
}
},
"require-dev": {
"phpunit/phpunit": "^4.8"
Copy link
Contributor

Choose a reason for hiding this comment

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

you should at least allow 4.8 or 5.7. 4.8 is not properly supporting PHP 7, and is unmaintained.

@@ -2,7 +2,7 @@

namespace EmailReplyParser\Tests;

class TestCase extends \PHPUnit_Framework_TestCase
abstract class TestCase extends \PHPUnit\Framework\TestCase
Copy link
Contributor

Choose a reason for hiding this comment

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

this actually requires 4.8.35+, not any 4.8.

@willdurand willdurand merged commit dc8e0ee into willdurand:master May 22, 2017
@kipit
Copy link
Contributor Author

kipit commented May 22, 2017

@willdurand For my understanding, how did you find the good recipe for a stronger .travis.yml?

@willdurand
Copy link
Owner

I merged a similar PR (willdurand/Hateoas#249) from @goetas before :)

@goetas
Copy link

goetas commented May 23, 2017

:)

@glensc
Copy link
Collaborator

glensc commented Nov 25, 2017

btw, i wrote this "doc" some time ago to make travis run phpunit for 5.3-7.3: eventum/rpc#2

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.

6 participants