Does not use Chrome on Windows on WSLv2

fabioxgn commented 3 years ago

Fixes #192

This PR https://github.com/titusfortner/webdrivers/pull/187 changed WSL detection to detect both WSLv1 and WSLv2 as running in WSL and use Chrome on Windows for both versions. But it doesn't work on WSLv2 out of the box.

WSL2 networking is different from WSL1 and you can't connect from the WSL2 Linux on Chromedriver running on Windows using 127.0.0.1:9515, see: https://github.com/microsoft/WSL/issues/4619

This PR changes the wsl detection to be explicity about v1 and only uses Chromedriver on Windows if it is WSLv1, as it doesn't work on WSLv2.

urubatan commented 3 years ago

This PR will also close #197

kapoorlakshya commented 3 years ago

@fabioxgn Thank you for this PR! Just to confirm, this PR will:

Stop webdrivers from finding Chrome binary on the Windows filesystem when using WSLv2. WSLv2 will be treated as Linux and only the Chrome binary on the Linux filesystem will be detected.
Chrome on both the Windows and Linux filesystem will continue to be found on WSLv1. So no change from the current behavior (v4.4.2) here.

Is that correct?

@urubatan Have you already tested this PR to confirm #197 will get fixed? Would like to be sure before we close both #192 and #197 when this PR gets merged.

fabioxgn commented 3 years ago

@fabioxgn Thank you for this PR! Just to confirm, this PR will:

Stop webdrivers from finding Chrome binary on the Windows filesystem when using WSLv2. WSLv2 will be treated as Linux and only the Chrome binary on the Linux filesystem will be detected.

Chrome on both the Windows and Linux filesystem will continue to be found on WSLv1. So no change from the current behavior (v4.4.2) here.

Is that correct?

Exactly 👍

urubatan commented 3 years ago

I just tested and the PR do not fix the issue

fabioxgn commented 3 years ago

I just tested and the PR do not fix the issue

What cat /proc/version returns when running on Docker with WSL2?

urubatan commented 3 years ago

I just tested and the PR do not fix the issue

What cat /proc/version returns when running on Docker with WSL2?

cat /proc/version Linux version 4.19.128-microsoft-standard (oe-user@oe-host) (gcc version 8.2.0 (GCC)) #1 SMP Tue Jun 23 12:58:10 UTC 2020

I found a resource (https://askubuntu.com/questions/1177729/wsl-am-i-running-version-1-or-version-2) saying that a sure way to check WSL v2 is to check if the linux version is 4.19 or above, I'm writing a PR with this in mind

fabioxgn commented 3 years ago

I just tested and the PR do not fix the issue

What cat /proc/version returns when running on Docker with WSL2?

cat /proc/version Linux version 4.19.128-microsoft-standard (oe-user@oe-host) (gcc version 8.2.0 (GCC)) #1 SMP Tue Jun 23 12:58:10 UTC 2020

I found a resource (https://askubuntu.com/questions/1177729/wsl-am-i-running-version-1-or-version-2) saying that a sure way to check WSL v2 is to check if the linux version is 4.19 or above, I'm writing a PR with this in mind

But with that output, it should work with this PR. Did you test with the wsl2 branch of my fork or with the master branch? See: https://github.com/titusfortner/webdrivers/pull/196/files#diff-c208b3bc55384e84ed6c4fc43d0a40f066e79fc93ce3825cfa0b946ad8aaa96aR153

It should detect your system as Linux and not WSL as microsoft did not return with caption M.

urubatan commented 3 years ago

@fabioxgn and @titusfortner my bad, the PR works, I pointed my Gemfile to @fabioxgn master branch, not to his wsl2 branch.

Sorry for the mistake, please merge this PR and ignore mine;

PS.: @fabioxgn might be a good idea to include the "false WSL test" check in your PR line this one https://github.com/urubatan/webdrivers/commit/c801b14d77c6877b6a7fd9758c5d3aef8d214331#diff-ac7f784131a2d9a0da98400f0909e0ab274625c5ca268516e24e98c4bcb46201R5

fabioxgn commented 3 years ago

@fabioxgn and @titusfortner my bad, the PR works, I pointed my Gemfile to @fabioxgn master branch, not to his wsl2 branch.

Sorry for the mistake, please merge this PR and ignore mine;

PS.: @fabioxgn might be a good idea to include the "false WSL test" check in your PR line this one urubatan@c801b14#diff-ac7f784131a2d9a0da98400f0909e0ab274625c5ca268516e24e98c4bcb46201R5

@urubatan There's a test for this: https://github.com/titusfortner/webdrivers/pull/196/files#diff-ac7f784131a2d9a0da98400f0909e0ab274625c5ca268516e24e98c4bcb46201R27

But my PR uses the word "Microsoft" to detect if is wsl1, as in wsl2 it is lowercase.

kapoorlakshya commented 3 years ago

@fabioxgn @urubatan Perfect! Thanks for confirming.

I'm closing #199 and merging this.

titusfortner / webdrivers

Does not use Chrome on Windows on WSLv2 #196