liuggio / fastest

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

Forward PATH to child processes #24

Closed Seldaek closed 6 years ago

Seldaek commented 9 years ago

Without this I can't get it to run at all, as it doesn't find php or phpunit or any command I give it. Not sure if it's the correct fix..

liuggio commented 9 years ago

Sorry for the late reply, I was on holiday... I need to investigate, but what $_server['path'] is? Which OS are you using?

Seldaek commented 9 years ago

Windows and $_SERVER['PATH'] is my PATH environment variable.

ioleo commented 8 years ago

This should be fixed by #52

@Seldaek can you retest and confirm?

Seldaek commented 8 years ago
$ find tests/ -name "*Test.php" | ../fastest/fastest
- 87 shuffled tests into the queue.
- Will be consumed by 4 parallel Processes.

87/87 [============================] 100%  1 sec 4.0 MiB

     87 failures.
[1] tests/Composer/Test/Repository/CompositeRepositoryTest.php'bin' is not recognized as an internal or external command,
operable program or batch file.

All tests fail with that same message. Sounds like it tries to execute "bin" as a command, not sure why and I don't really have the motivation to debug this right now.. Have you tried it on windows at all? :)

ioleo commented 8 years ago

@Seldaek without the -c option, the default command is bin/phpunit {}

for Windows you need to change that to bin\phpunit {}

so try with find tests/ -name "*Test.php" | ../fastest/fastest -c "bin\phpunit {}"

ill make a PR with a fix for that

Seldaek commented 8 years ago
[3] tests/Composer/Test/Downloader/PerforceDownloaderTest.php'phpunit' is not recognized as an internal or external command,
operable program or batch file.

$ which phpunit
/c/Users/seld/AppData/Roaming/Composer/vendor/bin/phpunit
Seldaek commented 8 years ago

Seems like that is a similar issue to what I was having when I created this PR :)

ioleo commented 8 years ago

@Seldaek you must edit your variable_order in php.ini to add E flag

see readme

PS. I've discovered and added it to readme yesterday :P

Seldaek commented 8 years ago

Well.. sorry but I don't think I should have to tweak my variable_order to make this work. It's just not nice user experience :) $_SERVER['PATH'] or getenv('PATH') work fine without having the E in var order. Can't fastest just use that?

ioleo commented 8 years ago

@Seldaek fastest uses $_ENV variable to pass all enviroment variables (along with PATH) to the child process. By default php has variable_order set to EGPCS (so, haveing E flag is the default). However some Windows PHP stacks (like EasyPHP) have it set to GPCS by default.

Without E flag, the $_ENV variable is empty.

Seldaek commented 8 years ago

Actually no, EGPCS is the default if not specified, but php.ini-production and php.ini-development in php7 both ship without E.

; Default Value: "EGPCS"
; Development Value: "GPCS"
; Production Value: "GPCS";
; http://php.net/variables-order
variables_order = "GPCS"

proc_open's env argument should just be set to null to forward current env (docs say An array with the environment variables for the command that will be run, or NULL to use the same environment as the current PHP process).

ioleo commented 8 years ago

@Seldaek fastest needs to add some custom ENV variables per process (so it cannot set them in the ENV) and the part responsible for this is Process/EnvCommandCreator.php#L16

To also include current process ENV variables it uses $_ENV. So we can't set it to null.

Seldaek commented 8 years ago

Hmm ok.. you could probably just use putenv() to set your new env vars and then use null to forward it all though?

liuggio commented 8 years ago

Hi I see two problems here:

  1. the "PATH" env that is accesible only via getenv('PATH')
  2. decide which is (or how to get) the standard test command to execute on windows.

for the first maybe just adding the PATH with getenv here: https://github.com/liuggio/fastest/blob/master/src/Process/EnvCommandCreator.php#L17

for the second I'm not a Windows user but maybe if is well configured we could just leave phpunit /phpunit.de/#installation.phar.windows

liuggio commented 8 years ago

/c @loostro

ioleo commented 8 years ago

@Seldaek sounds good, i'll test that

ioleo commented 8 years ago

Should be fixed by #58.

ioleo commented 8 years ago

Reopening, as #58 has been reverted due to some unexpected side effects.

francoispluchino commented 6 years ago

This PR can be closed, because already fixed.

DonCallisto commented 6 years ago

@francoispluchino have you tested it?

francoispluchino commented 6 years ago

Yes, and it's tested in the Liuggio\Fastest\Process\ProcessFactoryTest class.

DonCallisto commented 6 years ago

Thank you