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

PHPUnit as a dependency #31

Open
uuf6429 opened this issue Jul 30, 2018 · 14 comments
Open

PHPUnit as a dependency #31

uuf6429 opened this issue Jul 30, 2018 · 14 comments

Comments

@uuf6429
Copy link

uuf6429 commented Jul 30, 2018

Since this project is for PHPUnit and uses PHPUnit classes, doesn't it explicitly depend on PHPUnit?
In particular, shouldn't it depend on a specific PHPUnit version that is compatible?

This causes an issue when trying to support multiple versions of PHPUnit. This package does not downgrade to the version that works with PHPUnit.

@renatomefi
Copy link
Member

Hello @uuf6429, I took the freedom to answer directly the mentioned issue! Hope it helps

@uuf6429
Copy link
Author

uuf6429 commented Jul 31, 2018

Thanks! 3.1 would work, but only because of the common dependency on PHP. We can't use 3.0 for PHP 7.0 since it doesn't require it.

@renatomefi
Copy link
Member

If you have ^3.0 it will automatically solve it, by matching all the dependent versions

@uuf6429
Copy link
Author

uuf6429 commented Jul 31, 2018

Sorry to persist, but ^3.0 doesn't have any requirements... which is incorrect (same with any version, actually).
As it is right now, this package claims to be compatible with any PHPUnit version, even unreleased future ones...

@renatomefi
Copy link
Member

This library only depends on TestListener (https://github.com/sebastianbergmann/phpunit/blob/master/src/Framework/TestListener.php) and indeed "saying" it's compatible with all versions. Which can be improved for sure.
The real phpunit version limitations are coming from php-vcr/php-vcr and not here.

I'd say we could add a constraint saying it's compatible with php unit 5,6,7. But in the end you'll be limited to the php-vcr one!
Is that what you're looking for?

@uuf6429
Copy link
Author

uuf6429 commented Jul 31, 2018

This library only depends on TestListener...

To me that's a clear dependency. If someone installs this project but forgets to install PHPUnit, s/he ends up with a class extending and code referencing non-existent classes.

The real phpunit version limitations are coming from php-vcr/php-vcr and not here.

I do not understand your point here... php-vcr does not depend on phpunit to function, it uses phpunit only for tests. Right now the only PHPUnit version constraint comes from the user.

I'd say we could add a constraint saying it's compatible with php unit 5,6,7.

That's the best case scenario. Right now, it doesn't even suggest that you should have PHPUnit installed, nor does it say that it conflicts with older PHPUnit.

^3.1 works somewhat by coincidence: it relies on the fact that both PHPUnit, this project and the linked project depend on a specific PHP version. To be honest, I'd say this project shouldn't even care about PHP version... it should follow the same PHP version as the one PHPUnit and php-vcr (which you want to target) follow.

@renatomefi
Copy link
Member

Yes you're right! php-vcr doesn't depend on it 🤦‍♂️

The 3.1 was done on purpose to provide an easy update path for people using ^3.0, I also agree it isn't the best approach, check more here: #26

Please feel free to open a PR providing the phpunit dependency, luckily there's no conflicts so far since the mentioned interface remains the same for a long time

@uuf6429
Copy link
Author

uuf6429 commented Jul 31, 2018

No problem! I'll see if I can get this right for all major versions. Worst case we might end up with a few patch releases.

Edit: I've checked every notable version for compatibility with PHPUnit and PHP, here's the results:

tag pu4 pu5 pu6 pu7 php56 php70 php71 fix
1.0(.7) ✔️ ✔️ ✔️ ✔️ ✔️ too old?
1.1(.7) ✔️ ✔️ ✔️ ✔️ ✔️ 1.1.8 -
2.0(.0) ✔️ ✔️ ✔️ ✔️ ✔️ 2.0.1 -
3.0(.0) ✔️ ✔️ ✔️ ✔️ 3.0.1 #33
3.1(.0) ✔️ ✔️ ✔️ 3.1.1 #32
3.2(.1) ✔️ ✔️ ✔️ 3.2.2 #34

Note that the PHP checks are only about syntax, I didn't actually run the tests.

@uuf6429
Copy link
Author

uuf6429 commented Jul 31, 2018

@renatomefi I'd like to have two branches, one called 1.1 (branching from tag 1.1.7) and one 2.0 (branching from tag 2.0.0). When done, I can create my pull requests for both.

@ekeyte
Copy link

ekeyte commented Aug 7, 2018

@uuf6429 @renatomefi Hey there, just following up on this issue. Any updates here?

@uuf6429
Copy link
Author

uuf6429 commented Aug 11, 2018

@ekeyte I'm still waiting for the PRs (linked above) to be merged+released and for branches to the old versions to be created. I literally can't do anything more from my side.

@ekeyte
Copy link

ekeyte commented Dec 10, 2018

@uuf6429 @renatomefi Hey guys, are there any updates on this? This is holding us back from moving some big projects to 7.2. Anything I can do to help with this?

@uuf6429
Copy link
Author

uuf6429 commented Dec 11, 2018

I literally can't do anything more from my side.

Still applies..

@ekeyte
Copy link

ekeyte commented Dec 11, 2018

I literally can't do anything more from my side.

Still applies..

I know, bud. :(

@renatomefi: At your encouragement, @uuf6429 has opened a PR. Is there something holding you back from merging it that I could assist with? We'd love to see this resolved soon. Many thanks!

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

No branches or pull requests

3 participants