liuggio / fastest

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

phpunit not found: don't try to guess path to phpunit? #99

Closed tarlepp closed 6 years ago

tarlepp commented 7 years ago

It seems that this library relies that phpunit is found under bin directory.

https://github.com/liuggio/fastest/blob/97045f4f46fcecc934cc13e8721c4cc6c500a674/src/Process/ProcessFactory.php#L68-L73

Imho this should be looked under vendor/bin directory because composer will add that phpunit there and not under bin directory.

DonCallisto commented 7 years ago

@tarlepp have you a real use case for this?

Becase in composer.json we have the line

"config": {
        "bin-dir": "bin/"
    },

that should create a symlink between bin\phpunit and /vendor/phpunit/phpunit/phpunit

Let me know, thank you.

mtibben commented 7 years ago

If you're using composer the default actually is vendor/bin/phpunit and bin-dir is an override. Maybe it should check the default location first, or get it from composer config bin-dir ?

tarlepp commented 7 years ago

@DonCallisto eg. in my project https://github.com/tarlepp/symfony-flex-backend I need to add phpunit symlink to my project bin directory, which seems quite weird.

mtibben commented 7 years ago

@tarlepp you can invoke with fastest "vendor/bin/phpunit {};" to tell fastest what to execute

DonCallisto commented 7 years ago

If you're using composer the default actually is vendor/bin/phpunit and bin-dir is an override. Maybe it should check the default location first, or get it from composer config bin-dir ?

@mtibben I know it's an override but we got this in composer.json file so this should work straightforwardly.

Am I missing something?

DonCallisto commented 7 years ago

I've tried an example and, actually, it does not create a symlink itself. I don't think this should be a feature of this project as every vendor could be placed elsewhere and it could be difficult to find out where. I would suggest to stay with this configuration and run fastest as @mtibben suggested, specifying where phpunit bin is located.

If everyone agree, I'll close this.

alexislefebvre commented 7 years ago

@DonCallisto

I know it's an override but we got this in composer.json file so this should work straightforwardly.

Am I missing something?

I think that this part of composer.json is ignored when fastest is installed as a dependency.

DonCallisto commented 7 years ago

@alexislefebvre so, technically, it is useless.

alexislefebvre commented 7 years ago

It may be useful when working on this project or when the Travis CI run tests. We can remove this part and see if tests are broken or not.

DonCallisto commented 7 years ago

@alexislefebvre well, you're right, maybe for travis is a crucial parameter. We can try BTW.

However, do you agree with no modification to public static function getDefaultCommandToExecute()?

I suppose it's not a responsibility of this library to discover where the executable is, especially if you can specify it in CLI.

alexislefebvre commented 7 years ago

However, do you agree with no modification to public static function getDefaultCommandToExecute()?

I agree but if the removal breaks tests suites of fastest users (because the phpunit executable won't be found), this would be a BC break.

We shouldn't do this in a minor version but it can be done on a major version.

DonCallisto commented 7 years ago

@alexislefebvre let's keep everything as it is now

alexislefebvre commented 7 years ago

We can still improve the existing documentation by adding the path to phpunit in every example.

And also reopen and tag this issue as an idea for the future major release.

DonCallisto commented 7 years ago

@alexislefebvre could you auto assign this and complete the task of editing documentation + tagging and reopen the issue?

francoispluchino commented 6 years ago

With the 1.6.x-dev version, this issue is partially solved. It just misses checking in the vendor/bin directory. I will do a PR for this.

francoispluchino commented 6 years ago

Added in the v1.6.0 version.