Rely on own phpunit, not one from CI service #1995

keradus · 2016-06-15T22:07:48Z

SpacePossum · 2016-06-16T07:03:57Z

Thanks so much for picking this up!
I don't know much about the changes you made, I simply don't have the experience :)
I'll try to learn a bit about it, but looks good!

GrahamCampbell · 2016-06-16T08:47:33Z

composer.json

@@ -25,7 +25,8 @@
        "sebastian/diff": "~1.1"
    },
    "require-dev": {
-        "satooshi/php-coveralls": "0.7.*@dev"
+        "phpunit/phpunit": "@stable",


Wildcards are a bad idea.

@stable is exactly the same as *

This of course breaks prefer lowest too.

Perhaps ~4.0|~5.0 will work fine here?

👍 for ^4|^5 !

keradus · 2016-06-16T10:11:31Z

@SpacePossum please temporary merge this PR into your once that has problems with AppVeyor and let see if it will help

GrahamCampbell · 2016-06-16T10:15:45Z

.travis.yml

    - 'if [ "$SYMFONY_VERSION" != "" ]; then sed -i "s/\"symfony\/\([^\"]*\)\": \"[^\"]*\"/\"symfony\/\1\": \"$SYMFONY_VERSION\"/g" composer.json; fi'

 install:
+    - travis_retry composer global require hirak/prestissimo


This actually ends up slower surely? It takes just as long to install this globally as it does to install the rest.

You mean about the time to insstall this library?

Dependencies for this one is very light, it should not be a problem.

@keradus I would ever say that the travis_retry should not be here.

I see no reason to drop it.

travis_retry is good for long with no output jobs.

This command would not take more than 10 minutes to execute.

Plus, travis_retry drop the original output of the command, harder to read.

This is as you want, but I suggest you to remove this. 👍

SpacePossum · 2016-06-16T10:21:59Z

thanks, done over @ #1949 (please note I made changes because that PR targets master, but as a POC it should be useful for now)

soullivaneuh · 2016-06-16T10:46:40Z

composer.json

@@ -25,7 +25,8 @@
        "sebastian/diff": "~1.1"
    },
    "require-dev": {
-        "satooshi/php-coveralls": "0.7.*@dev"
+        "phpunit/phpunit": "^4.5|^5",


👎

If we are able to not make it dependents, don't do it.

This will install a lot of dependencies, making composer longer to resolve.

Plus, all developer does not work with vendor/bin/phpunit.

What about directly download the .phar on travis?

Example: https://github.com/sonata-project/dev-kit/blob/a4574f4ff3428bdd89ecf1c2f4f0190227abb250/project/.travis/install_test.sh#L4-L13

I don't like fetching some deps by Composer and some not. You don't like it? Cool. You are not enforced to download dev deps.
Composer is nicer way to describe required version. like not supporting 4.1

I don't like fetching some deps by Composer and some not.

As you want but IMHO, it's more a tool than a "dependency".

You don't like it? Cool. You are not enforced to download dev deps.

Not really true for all projects. Some can require dev packages other than phpunit and we can select which one we want to install.

By installing with composer, you get dependencies described by phpunit, not locked by composer.lock like for .phar. This mean you increase the issue risk to get PHPUnit not working because of a sub-dependency update.

So this is not only a matter of taste for me. 😉

Composer is nicer way to describe required version. like not supporting 4.1

Well, if you not support it, tests will fail. :-)

tool could be a dependency

to develop a project you should have a phpunit. you may have global one - cool, but we don't promise that our project is compatible with it.

theoretically, but such an edge case...

it;s better to say "this one will work, you may use it" than "some phpunit could work, we dont know, try, and if it fails - well... we dont know which version works"

it;s better to say "this one will work, you may use it" than "some phpunit could work, we dont know, try, and if it fails - well... we dont know which version works"

Or simply rely on the last stable version of PHPUnit. 😉

theoretically, but such an edge case...

Are you talking about the risk to get PHPUnit not working because of a sub-dependency update?

Not such an edge case as you think. 😉

Anyway, at least I have an answer. :-)

keradus · 2016-06-16T10:56:09Z

So, this fixes AppVeyor bug of #1949 @SpacePossum
(related Travis fail is due to quick cherry-pick without solving conflicts on .travis.yml)

SpacePossum · 2016-06-16T11:04:49Z

Indeed, AppVeyor is happy, so 🚢 this :)
(Maybe in some other life I try to figure out want went wrong with the previous setup)

keradus · 2016-06-16T11:06:30Z

AppVeyor break it's PHPUnit, I remember that it happens once with Travis as well.

soullivaneuh · 2016-06-17T08:23:10Z

So no concern about my last comment? #1995 (comment) :-/

FYI, if you made this PR to fix this error: https://ci.appveyor.com/project/keradus/php-cs-fixer/build/dev-2529

Please not this is not related at all. This was an issue from PHPUnit: sebastianbergmann/phpunit#2219

keradus · 2016-06-17T08:45:20Z

@soullivaneuh answered.
To be honest I see no value in continuing this topic, it won't bring any new value. I like to be consistent and have one way of dealing with deps, you like to be minimalistic. I see good and bad points of both approaches.

If you have some power and willing please focus on something important, like #1119

keradus and others added 2 commits June 16, 2016 00:07

Rely on own phpunit, not one from CI service

d5832f9

Update .travis.yml

509c4dc

GrahamCampbell reviewed Jun 16, 2016
View reviewed changes

Update composer.json

236dc72

GrahamCampbell reviewed Jun 16, 2016
View reviewed changes

SpacePossum mentioned this pull request Jun 16, 2016

Added DeclareStrictTypesFixer, introduce options for HeaderCommentFixer #1949

Merged

soullivaneuh reviewed Jun 16, 2016
View reviewed changes

soullivaneuh mentioned this pull request Jun 16, 2016

Travis: Composer parallel installs with hirak/prestissimo plugin sonata-project/dev-kit#173

Closed

keradus added this to the v1.11.5 milestone Jun 16, 2016

keradus added RTM Ready To Merge and removed RTM Ready To Merge labels Jun 16, 2016

keradus closed this in feeca5a Jun 17, 2016

keradus deleted the 1.11_phpunit branch June 17, 2016 08:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rely on own phpunit, not one from CI service #1995

Rely on own phpunit, not one from CI service #1995

keradus commented Jun 15, 2016

SpacePossum commented Jun 16, 2016

GrahamCampbell Jun 16, 2016

GrahamCampbell Jun 16, 2016

GrahamCampbell Jun 16, 2016

GrahamCampbell Jun 16, 2016

keradus Jun 16, 2016

keradus commented Jun 16, 2016

GrahamCampbell Jun 16, 2016

soullivaneuh Jun 16, 2016

keradus Jun 16, 2016

soullivaneuh Jun 16, 2016

SpacePossum commented Jun 16, 2016

soullivaneuh Jun 16, 2016 •

edited

keradus Jun 16, 2016

soullivaneuh Jun 16, 2016

keradus Jun 17, 2016

soullivaneuh Jun 17, 2016

keradus commented Jun 16, 2016

SpacePossum commented Jun 16, 2016

keradus commented Jun 16, 2016 •

edited

soullivaneuh commented Jun 17, 2016 •

edited

keradus commented Jun 17, 2016

Rely on own phpunit, not one from CI service #1995

Rely on own phpunit, not one from CI service #1995

Conversation

keradus commented Jun 15, 2016

SpacePossum commented Jun 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keradus commented Jun 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SpacePossum commented Jun 16, 2016

soullivaneuh Jun 16, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keradus commented Jun 16, 2016

SpacePossum commented Jun 16, 2016

keradus commented Jun 16, 2016 • edited

soullivaneuh commented Jun 17, 2016 • edited

keradus commented Jun 17, 2016

soullivaneuh Jun 16, 2016 •

edited

keradus commented Jun 16, 2016 •

edited

soullivaneuh commented Jun 17, 2016 •

edited