Skip to content

Require phpunit on dev deps #11

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

Closed
soullivaneuh opened this issue Jan 21, 2018 · 8 comments
Closed

Require phpunit on dev deps #11

soullivaneuh opened this issue Jan 21, 2018 · 8 comments

Comments

@soullivaneuh
Copy link

soullivaneuh commented Jan 21, 2018

PHPUnit should not be a mandatory requirement.

Some users would like to use PHPUnit phar global installation and don't want to have to get the whole dependencies of PHPUnit and itself on the project vendor dir.

What is the goal of this requirement? I don't see any direct class usage so far.

UPDATE: BTW, it also mandatory to require also phpstan from plugins? What is the usage if we use the docker image or the shim repository?

Regards

@ondrejmirtes
Copy link
Member

Hi, there's a similar request on phpstan-doctrine: phpstan/phpstan-doctrine#9 I'm still evaluating this. The problem is that PHPStan needs to see PHPUnit classes to analyse code correctly. So if you don't have them autoloaded, you need to add your PHPUnit installation to autoload_directories/autoload_files.

phpstan-shim is fine since it has phpstan/phpstan in compoer.json replace: https://github.com/phpstan/phpstan-shim/blob/master/composer.json#L8 So when you're installing an extension alongside it, it does not install phpstan/phpstan but reuses the shim version.

Docker: I'm not sure about how to solve this one, I'll ask the author of the images.

@soullivaneuh
Copy link
Author

phpstan-shim is fine since it has phpstan/phpstan in compoer.json replace:

You didn't understand IMHO, I'm talking about global installation like composer global require phpstan/phpstan-shim, or a .phar.

If we require one of the plugins on our project dependencies, we will have phpstan installed (and phpunit) for this case.

Sample: symfony/phpunit-bridge works with PHPUnit but do not directly require it. It let the choice to the user. 👍

So if you don't have them autoloaded, you need to add your PHPUnit installation to autoload_directories/autoload_files.

Well, not a big deal if this is configurable. We just have to add the correct error message. ;-)

@o5
Copy link

o5 commented Feb 2, 2018

👍 for dev deps.

I wanted to use PHPUnit 7 in our global composer dependencies (in Docker base image) and it failed:

Problem 1
    - Installation request for phpstan/phpstan-phpunit 0.9.3 -> satisfiable by phpstan/phpstan-phpunit[0.9.3].
    - phpstan/phpstan-phpunit 0.9.3 requires phpunit/phpunit ^6.3 -> satisfiable by phpunit/phpunit[6.3.0, 6.3.1, 6.3.x-dev, 6.4.0, 6.4.1, 6.4.2, 6.4.3, 6.4.4, 6.4.x-dev, 6.5.0, 6.5.1, 6.5.2, 6.5.3, 6.5.4, 6.5.5, 6.5.6, 6.5.x-dev] but these conflict with your requirements or minimum-stability.

I'll send PR with hotfix.

@o5 o5 mentioned this issue Feb 2, 2018
@mattgreenham
Copy link

I think this is the root of an issue we're having which causes an incompatibility with symfony/phpunit-bridge, as phpstan/phpstan-phpunit is pulling in a newer version of PHPUnit.

On a fresh Symfony 4.0.4 install with PHPStan, tests work fine but after requiring phpstan/phpstan-phpunit, PHPUnit 7 is installed and running tests throws the following error:

PHP Fatal error:  Declaration of Symfony\Bridge\PhpUnit\TextUI\TestRunner::handleConfiguration(array &$arguments) must be compatible with PHPUnit\TextUI\TestRunner::handleConfiguration(array &$arguments): void in /path/to/file/my_project/vendor/symfony/phpunit-bridge/TextUI/TestRunner.php on line 28

@ondrejmirtes
Copy link
Member

I aim to solve this for all extensions by removing phpunit/phpunit (and other similar dependencies) from require and just adding conflict with unsupported versions.

@soullivaneuh
Copy link
Author

@ondrejmirtes Thanks, it's a great news! 👍

I'm currently stuck with a PHPUnit plugin upgrade because the BC tricks detect classes with the composer dep phpunit and not the .phar.

Having this dependency removed will solve that.

@ondrejmirtes
Copy link
Member

This is solved for a long time.

@github-actions
Copy link

github-actions bot commented May 1, 2021

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants