search

liuggio / fastest

Simple parallel testing execution... with some goodies for functional tests.
MIT License
475 stars 65 forks source link

Added compatibility with Symfony 5 components #146

Closed webhdx closed 4 years ago

webhdx commented 4 years ago

Since #143 is not really moving forward I'm making new pull request. I've aligned the code to suggestions from @DonCallisto. Since it is no longer possible to mock static methods in phpunit (ref. https://github.com/sebastianbergmann/phpunit-documentation/issues/77) I didn't want to go into the route of using another Mock framework. It would be just better to rely on Travis so it tests using different Symfony versions. I think this is the case now so it's covered.

I've also fixed ProcessManagerTest by changing all commands to arrays since it's supported in all symfony/process versions.

gronostajo commented 4 years ago

I'm trying to use your code. I've added Doctrine config from README and I'm getting errors at ConnectionFactory:30. I think it's because I'm using a connection string (doctrine.dbal.connections.*.url) so I don't have neither dbname or master in $params.

webhdx commented 4 years ago

@gronostajo I need more info how to reproduce your setup.

gronostajo commented 4 years ago

@webhdx I believe it's standard Symfony/Doctrine setup since Symfony 4.0. In doctrine.yaml I don't have separate dbname+host+user+password but a single url (provided through an environment variable, but that should be irrelevant). So there's a single string like this: postgres://user:password@host:5432/dbname

gronostajo commented 4 years ago

Here's a Doctrine config created by Flex that should reproduce this issue:

https://github.com/symfony/recipes/blob/4293dc6cb23feee18258a5239494292cc7cbb251/doctrine/doctrine-bundle/2.0/config/packages/doctrine.yaml

kocoten1992 commented 4 years ago

gently ping @DonCallisto

DonCallisto commented 4 years ago

Sorry for being late on this. I don't think that's gonna work. Basically if you take a look at

https://github.com/liuggio/fastest/pull/146/files#diff-9a08194c7e449f5d3951cb3814bf513aR56-R60

you can see that static function and constructor are called with same parameter (type). That's cleary an error, as you can check the implementation of Process before sf5: you clearly see that constructor accept only array wereas the static function accepts only string. I think that tests are false positives here.

Take a look at previous comments I've done on #143

https://github.com/liuggio/fastest/pull/143#issuecomment-559963043 https://github.com/liuggio/fastest/pull/143#issuecomment-559968777

Maybe this idea is good, even if I would go with a "common" solution (like cast all to array or cast all to strings). Maybe this build was failing due to other reasons (like how fastest is executed? Didn't checked in depth).

Moreover I can't understand if https://github.com/liuggio/fastest/pull/146#issuecomment-602613971 is realated or not.

matlar83 commented 4 years ago

+1 for this PR :-) Can't wait to start using Symfony 5

matlar83 commented 4 years ago

hi @webhdx, do you have any update about this PR? Can I help in any way?

DonCallisto commented 4 years ago

See https://github.com/liuggio/fastest/pull/149

DonCallisto commented 4 years ago

@webhdx @gronostajo @kocoten1992 @matlar83 check #149 please and give it a try and a feedback. Thanks.