Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Windows env fix#1309

Closed
devcer wants to merge 2 commits into
firefox-devtools:masterfrom
devcer:windows_env_fix
Closed

Windows env fix#1309
devcer wants to merge 2 commits into
firefox-devtools:masterfrom
devcer:windows_env_fix

Conversation

@devcer

@devcer devcer commented Nov 24, 2016

Copy link
Copy Markdown
Contributor

Associated Issue: #1248

Summary of Changes

*edited firefox-driver.js to run 'yarn start firefox'

Test Plan

Tell us a little a bit about how you tested your patch.

Example test plan:

*Navigate to debugger.html folder from command prompt from windows
*Execute "yarn start firefox" to start the server
*Open browser and go to localhost:8000

Output: debugger.html is up and running

Screenshots/Videos (OPTIONAL)

yarn start firefox vs yarn run firefox

yarn2

@jasonLaster

Copy link
Copy Markdown
Contributor

CC @MoLow

@MoLow

MoLow commented Nov 24, 2016

Copy link
Copy Markdown
Contributor

@jasonLaster whats this?

@jasonLaster

jasonLaster commented Nov 24, 2016 via email

Copy link
Copy Markdown
Contributor

@MoLow

MoLow commented Nov 28, 2016

Copy link
Copy Markdown
Contributor

Hi, sorry for the delay in answering. In the meanwhile i tested on a computer with no admin priviligaes, so I could not install yarn, neither update node to > 6.9.0, so I tried running it directly, and this is the results:

image

I'l try it later today on my computer using npm run/yarn start/yarn run.

BTW, I havent tried yet, but this maybe a simple solution: https://www.npmjs.com/package/npm-run-path

@clarkbw

clarkbw commented Nov 28, 2016

Copy link
Copy Markdown
Contributor

yarn start firefox vs yarn run firefox

I believe with yarn start firefox yarn is just actually starting the server and it actually passes 'firefox' as an argument to the server.

When you run yarn run firefox you're seeing the issue we seem to be having running Firefox on windows.

@clarkbw

clarkbw commented Nov 28, 2016

Copy link
Copy Markdown
Contributor

Ok, and I just looked into this further. I think the first solution is to update our instructions.

The error you're seeing in that screenshot is because you don't have the geckodriver.exe file in your local path. Here are some quick instructions so solve this:

  • Open https://github.com/mozilla/geckodriver/releases/latest
  • Download the build file that matches your system (like geckodriver-v0.11.1-win64.zip)
  • Create a local bin directory by doing mkdir ~/bin
  • Extract the gecokdriver.exe file into the bin directory you just created

Now when you run yarn run firefox Firefox should start up.

I am seeing a problem where you I don't believe the debugger server is running, looking into that now.

@MoLow

MoLow commented Nov 28, 2016

Copy link
Copy Markdown
Contributor

@clarkbw well, the question is if this could be done automatically on npm install or on npm run firefox, since the exe is installed to node_modules\geckodriver, and it only needs to get into the path in runtime.

@clarkbw

clarkbw commented Nov 28, 2016

Copy link
Copy Markdown
Contributor

could be done automatically on npm install or on npm run firefox, since the exe is installed to node_modules\geckodriver, and it only needs to get into the path in runtime.

Yes, sorry. What I have above shouldn't actually be the solution, just short term. On the Mac this is done automatically and it should be on Windows as well.

@jasonLaster

Copy link
Copy Markdown
Contributor

@clarkbw perhaps look at npm run path issues for windows

@clarkbw

clarkbw commented Nov 28, 2016

Copy link
Copy Markdown
Contributor

Oh, hmm. I'm seeing node_modules/.bin/geckodriver where i believe it should be node_modules/.bin/geckodriver.exe

@clarkbw

clarkbw commented Nov 28, 2016

Copy link
Copy Markdown
Contributor

Or, actually geckodriver.cmd

@clarkbw

clarkbw commented Nov 28, 2016

Copy link
Copy Markdown
Contributor

The .bin/ directory has files with the .cmd extension, which run just fine. I found that if I change this line with a s/exe/cmd/ then it works just fine.

https://github.com/SeleniumHQ/selenium/blob/ba56ad1ae0b98a1fe1efdd2163c62fc847950178/javascript/node/selenium-webdriver/firefox/index.js#L288

@clarkbw

clarkbw commented Nov 28, 2016

Copy link
Copy Markdown
Contributor

I've been trying out this solution, creating the driver manually, but still not having success:

  let options = new firefox.Options();
  let service = new firefox.ServiceBuilder(__dirname + '\\..\\node_modules\\.bin\\geckodriver.cmd').build();

  options.setProfile(firefoxProfile());
  options.setBinary(firefoxBinary());

  const driver = new firefox.Driver(options, service);

It finds the local gecko driver but errors out elsewhere before running anything.

@MoLow

MoLow commented Nov 28, 2016

Copy link
Copy Markdown
Contributor

well, now I ran it on another machine, and it seems the correct directory is prepended to PATH - only when running via npm run firefox;

this the result of echo $PATH and adding console.log(process.env.PATH); to firefox-driver.js

image

however, I created a file called "path.js" which only contains console.log(process.env.PATH);, and the directory is not prepended:
image

when I changethe firefox command in package.json to node path.js, the path is prepended again:

image

@MoLow

MoLow commented Nov 28, 2016

Copy link
Copy Markdown
Contributor

Mabe the simplest solution is to add these lines to firefox-driver.js?:

var path = require('path'); process.env.PATH = path.resolve(__dirname, '../node_modules/.bin') + ';' + process.env.PATH;

is this a BAD idea?
would it work in linux?

@clarkbw

clarkbw commented Dec 9, 2016

Copy link
Copy Markdown
Contributor

is this a BAD idea?
would it work in linux?

As I understand it, the .bin/ path is being added correctly, however it is looking for geckodriver.exe on windows and that .exe isn't found in that .bin/ path.

I think you need to do something like this:

if (isWindows) { 
var path = require('path'); 
process.env.PATH = path.resolve(__dirname, '../node_modules/geckodriver/bin') + ';' + process.env.PATH; 
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants