-
Notifications
You must be signed in to change notification settings - Fork 322
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
Add Windows support #13
Conversation
return base_path('vendor\bin\phpunit.bat'); | ||
} | ||
|
||
return 'php vendor/bin/phpunit'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this not also use the base_path()
function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was from first version. I tried to keep all that is not connected with windows unchanged. If you have linux or macOS can you check if it work with base_path()
?
Works for me on Windows 10. 👍 |
For me too. Windows 10 64 Bit |
|
||
protected static function initChromeDriver(){ | ||
static::$chromeProcess = (new ProcessBuilder()) | ||
->setPrefix(realpath(__DIR__.'/../bin/chromedriver-' . static::getOSSuffix() . '.exe')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That .exe
extension should happen only when is WINNT
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! I have moved it to getOSSuffix()
so it should work for non WINNT platforms
does not work on windows 2.0 pls help |
@Deejavu What's Windows 2.0? |
@KKSzymanowski Windows from 1987 |
Breaks on my Mac: sh: php vendor/bin/phpunit: No such file or directory ... Worked before. |
Also please revert all unrelated spacing changes that were made. |
I have no way to test it on a Mac. Can you give us more info? |
@taylorotwell Can you check it now? I have made rebase to add #10 fix. I will try to check it on ubuntu |
Have checked on Ubuntu 16.04 x64. All work fine |
I have managed to install Mac OS X and I can confirm that the newest version works. |
@taylorotwell have you tested it? Is it works for you now? |
I too am curious. I'd like to see this merged as soon as possible. Can we help you with anything? |
It works on Windows 10 |
@spyric could you please fix the conflict? It might be "better" for @taylorotwell to merge it ;-) |
@alex-LE Conflict is resolved |
Can confirm that this made dusk work on my Arch Linux desktop as well. |
Confirmed to work on Windows 10. What guys you are getting as execution time? Because I am getting closer to six seconds just with the provided example. I was watching the Laracast video about Dusk and Jeffrey has something like less than one second. PHPUnit 5.7.8 by Sebastian Bergmann and contributors.
. 1 / 1 (100%)
Time: 5.72 seconds, Memory: 8.00MB
OK (1 test, 1 assertion) |
I still don't understand why unrelated spacing changes are being made. It halts the inclusion of this PR. |
The rest of the Laravel framework has doc-blocks on every method. |
Cleaned up all the spacing and other formatting issues. Thanks for the help with Windows debugging! |
I need help. I have laravel 5.4 and windows 10. if I execute php artisan dusk I get: |
@@ -18,17 +19,15 @@ | |||
*/ | |||
public static function startChromeDriver() | |||
{ | |||
if (PHP_OS === 'Darwin') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`
Hi! how can I make it work on Windows 7 ? |
That's just a warning. |
Well, maybe it's just a warning, but why |
Works for me on Windows 7. 👍 |
Works for me too, I copied the files from github |
This PR add Windows support.
I have no chance to test it on linux or mac, but hope It should work