PHPUnit 6

[simPod]

Let's go ahead and use PHPUnit 6! 🎉

[DonCallisto]

Master branch tests are not failing (see this --> https://travis-ci.org/liuggio/fastest/builds/242026028)

[simPod]

Sure thing, I checked it before but for example this one fails for me on master

I believe it has to do something with my hw setup

I'll just fix FastestEnvironmentTest + some cleanup

[simPod]

It's done

[alexislefebvre]

I'm voting :-1: about the removal of PHPUnit 4 and 5. It should be possible to keep PHPUnit 5.7 at least, thanks to the compatibility layer, we used it on LiipFunctionalTestBundle: https://github.com/liip/LiipFunctionalTestBundle/pull/322/files

[DonCallisto]

It should be possible to keep PHPUnit 5.7 at least

Yes it is exactly was worried me most. If it is possible to keep PHPUnit 5.7 it would be a blast!

@simPod you also could keep php 5.6 for a while in pipe with PHP7

Moreover, please, squash all commits in one.

[simPod]

Ok, wasn't that hard as 5.7 is kinda forward compatible with 6

[DonCallisto]

Why is composer.lock included? I mean, in every project I run, I use to include it but I'm wondering - as I'm new to this project - why it was kept out "of the business" since now. Do you know why @alexislefebvre ?

[simPod]

Me too, as last edit of .gitignore was in 2014, I considered it as legacy-something

[DonCallisto]

@simPod so why did you included it? is there any particular reason?

[simPod]

Cuz it's recommended and makes sense. Everyone will use the same dependecy versions. It should be there.

[DonCallisto]

@simPod so that should be part of a different PR so we can evaluate (and accept) it separately.

Does this make sense to you?

[alexislefebvre]

@simPod

Cuz it's recommended and makes sense. Everyone will use the same dependecy versions. It should be there.

Who recommend this? For what reason? Can you please give a deeper explanation? Lots of Composer do not version their lock file.

IIRC, the composer.lock is only readen when developing this project (not when it's installed in vendor/). Forcing a specific version may be actually worse because it would limit the dependencies to specific versions, we need to allow multiple versions of the dependencies to be used when developing and testing this package.

[simPod]

@alexislefebvre FYI i have removed it from this PR

...however...

https://getcomposer.org/doc/01-basic-usage.md#commit-your-composer-lock-file-to-version-control

It doesn't force specific version, it locks it. So when you or any other collabolator runs composer install, he gets the same exact version as the one who last commited it. CI will everytime run the same version.

When you don't have composer.lock, ^0.4 simply stands for 0.4.0, 0.4.1, 0.4.2 etc. So the highest version that matches your constraint gets installed.

And here's the issue. Let's say you are using PHPUnit as ^6.2. Running composer install installs version 6.2.2. You create PR, merge into master, build is passing, everyone is happy. But then Sebastian comes home drunk and releases broken 6.2.3. If you had composer.lock and locked to exact commit at 6.2.2, your build would be still passing and you wouldn't even notice the next version of PHPUnit is broken. But no, you don't have composer.lock so immediatelly every new build gets PHPUnit 6.2.3. Master branch, yet untouched, starts failing. Your whole project is suddenly broken and doom comes upon us all.

composer update <vendor>/<package> updates the locked package version so it doesn't force you to use it. It just gives you more control.

[DonCallisto]

And here's the issue. Let's say you are using PHPUnit as ^6.2. Running composer install installs version 6.2.2. You create PR, merge into master, build is passing, everyone is happy. But then Sebastian comes home drunk and releases broken 6.2.3. If you had composer.lock and locked to exact commit at 6.2.2, your build would be still passing and you wouldn't even notice the next version of PHPUnit is broken. But no, you don't have composer.lock so immediatelly every new build gets PHPUnit 6.2.3. Master branch, yet untouched, starts failing. Your whole project is suddenly broken and doom comes upon us all.

This is an interesting point of view but think about the following scenario

Let's say we have ^6.2 and composer.lock committed. I would like to be sure that every version of 6.* will run without problem because there could be someone using our library/bundle that uses PHP 6.3 and I want to be noticed if 6.3 broke all the things.

So I'm still not sure about including composer.lock; I mean, if the project is a "private" one, I'm pretty sure that committing it is the only way to guarantee safety between a team, but I'm not sure we should move in this way for OSS.

Could you please open a different PR so we can discuss it elsewhere?

PS.: @alexislefebvre does this PR LGTY? If so, could you please merge it and release a new (minor? I suppose should be a minor) tagged version?

[simPod]

Yup but that's something you shouldn't do on repository level. It's similar to writing integration tests, that test that some 3rd party API is working. It's not something you can influence and you shouldn't be testing this. If 3rd party API goes down, your whole test suite starts failing. They should be writing tests for it, not you. If someone finds out PHPUnit 6.3 broke things, he will open issue/submit PR.

I'll open next PR when this is merged

[DonCallisto]

@alexislefebvre can I merge this? Do you agree with a new tagged (minor) version (1.4.1)?

[alexislefebvre]

@DonCallisto Yes, this PR looks good to me. I don't know this project perfectly so I didn't want to publish a tag on my own. You can do it. :)

[alexislefebvre]

Thanks a lot @simPod and @DonCallisto!