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

Adds back PHP_BINARY to phpunit process #240

Merged
merged 1 commit into from
Apr 20, 2017

Conversation

jhoff
Copy link

@jhoff jhoff commented Apr 20, 2017

Disclaimer: Not sure of the implications of this on windows, as I can't test it...

A while back a pull request was accepted that ensured PHP_BINARY was used for phpunit. Almost immediately after, that change was reverted in a refactor. This brings that fix back.

@taylorotwell
Copy link
Member

So why are we adding it back?

@jhoff
Copy link
Author

jhoff commented Apr 20, 2017

This change simply makes sure that phpunit is run with the same version of php that you executed artisan with. Without it, phpunit will try and resolve php from the current environments $PATH, but symfony's process executes the command in a fresh environment that doesn't inherit $PATH from the executed environment.

In our case, we are running tests with different versions of php depending on the project we're currently working with so we can't rely on $PATH to resolve the correct version for us.

@taylorotwell
Copy link
Member

So why was it reverted?

@jhoff
Copy link
Author

jhoff commented Apr 20, 2017

As far as I can tell, the original fix was accepted and then in the very next merge commit it was gone completely:

image

db89a68
da7508e ( specifically here:

public function handle()
)

If you trace back the commit parents on that second merge commit, it looks like the refactor was here:

00338d5#diff-9b77ed94c2c2c5083de8ec6cc456096eR49

I don't think it was intentionally reverted, just overlapping merge requests with outdated base commits.

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

Successfully merging this pull request may close these issues.

2 participants