SeleniumHQ / selenium-ide

Open Source record and playback test automation for the web.
https://selenium.dev/selenium-ide/
Apache License 2.0
2.79k stars 761 forks source link

electron-chromedriver folder #1524

Closed giuliohome closed 1 year ago

giuliohome commented 1 year ago

I followed the readme instruction on Ubuntu in Windwos WSL 2.

But I had to do the following

cp -r node_modules/electron-chromedriver/ packages/selenium-ide/node_modules/

(i.e. to copy the electron-chromedriver folder from the root's node_modules to the node_modules in the packages/seleniumide)

to make it work!

toddtarsi commented 1 year ago

Hm! That's strange to me. What version of yarn / NodeJS are you building on? We specifically build using the 'nohoist' flag from yarn here.

https://github.com/SeleniumHQ/selenium-ide/blob/74fa23e5a7db0ea1b95917e7901cc59367a37e96/packages/selenium-ide/package.json#L92

This should keep these packages from getting lifted up to the parent directory, but I've messed many things up before haha.

giuliohome commented 1 year ago

Hm, interesting. After a quick glance at the source, it seems you check the driver path here:

https://github.com/SeleniumHQ/selenium-ide/blob/3faf7291a91f5da242220ba3d8b64e54c790995e/packages/selenium-ide/src/main/session/controllers/Driver/start.ts#L67

which is resolving to

https://github.com/SeleniumHQ/selenium-ide/blob/3faf7291a91f5da242220ba3d8b64e54c790995e/packages/selenium-ide/src/main/session/controllers/Driver/start.ts#L36-L42

and I suspect it simply points to the the node_modules in the packages/selenium-ide at runtime (irrespectively of nohoist in yarn workspaces... and btw nohoist is deprecated in yarn3).

giuliohome commented 1 year ago

My proposed fix

$ git diff packages/selenium-ide/
diff --git a/packages/selenium-ide/src/main/session/controllers/Driver/start.ts b/packages/selenium-ide/src/main/session/controllers/Driver/start.ts
index 85a96393..c86b7e81 100644
--- a/packages/selenium-ide/src/main/session/controllers/Driver/start.ts
+++ b/packages/selenium-ide/src/main/session/controllers/Driver/start.ts
@@ -35,7 +35,7 @@ const getDriver = ({ browser, version }: BrowserInfo) =>
     ? path.resolve(
         path.join(
           __dirname,
-          '..',
+          '..','..','..',
           'node_modules',
           'electron-chromedriver',
           'bin',
toddtarsi commented 1 year ago

@giuliohome - Oh holy shit I'm using yarn v1 locally haha. 🤦 💀

I can't change the path to this exactly yet though because we also do asarUnpack flags here:

https://github.com/SeleniumHQ/selenium-ide/blob/74fa23e5a7db0ea1b95917e7901cc59367a37e96/packages/selenium-ide/package.json#L34

And since this builds out of the CI, I worry merging this will actually fully break the latest binary in alpha. I know I need more testing via spectron on this stuff btw. I intend to write a sanity test or two soon. Currently I'm using the example side files to do focused testing.

giuliohome commented 1 year ago

By the way, if I go to selenium-ide/packages/side-runner and I run

pnpm start --  /.../myexample/selenium-ide.side

it first complains Error: The ChromeDriver could not be found on the current PATH.

If I put /selenium-ide/packages/selenium-ide/node_modules/electron-chromedriver/bin/ then the above error is solved but I still have a problem:

    SessionNotCreatedError: session not created: This version of ChromeDriver only supports Chrome version 100
    Current browser version is 106.0.5249.91 with binary path /usr/bin/google-chrome

Of course I can do instead

wget https://chromedriver.storage.googleapis.com/106.0.5249.61/chromedriver_linux64.zip
unzip chromedriver_linux64.zip

And voilà... my test starts also from side-runner (I don't understand how to set timeout, ignore ssl cert errors.. pass headless as arg.. but ok atm I'm temporary deleting the timeout at code level to manually ignore the certificate errors and I don't need headless now)

If that is the way to link the chromedriver (i.e. wget), then perfect, all is working as expected (in regard to side-runner). Out of my curiosity: am I using the ChromeDriver version 100 when I am recording the .side test? Is it coherent with the runtime version of side-runner?

toddtarsi commented 1 year ago

With regard to the side runner, yeah whatever works is good. That's one of those tools that is meant to be driver-ambiguous. The expected workflow tends to be something like this:

  1. Install your drivers onto $PATH (most CI systems do this for you ahead of time I believe)
  2. Install side-runner either locally or globally. npm i -g @seleniumhq/side-runner
  3. Run side-runner with your desired config. selenium-side-runner -c "goog:chromeOptions.args=[headless,no-sandbox] browserName=chrome" -t 15000 ./project.side
toddtarsi commented 1 year ago

Honestly, at some point I might rename the side-runner bin to like selenium-side-runner-4 or something like that, because Github installs selenium-side-runner v3 as a global bin and the name conflicts make it hard as hell to use v4

giuliohome commented 1 year ago

I am doing the following (*) from packages/side-runner but I think the capabilities are not passed (e.g. I see chrome in XLaunch, it is not headless)

 pnpm start -- -c "goog:chromeOptions.args=[headless,no-sandbox] browserName=chrome" /home/giulio/dso-demo/selenium-ide.side

> @seleniumhq/side-runner@4.0.0-alpha.15 start /home/giulio/selenium-ide/packages/side-runner
> node dist/bin.js "--" "-c" "goog:chromeOptions.args=[headless,no-sandbox] browserName=chrome" "/home/giulio/dso-demo/selenium-ide.side"

and I see a different result here with reference to what happens in packages/selenium-ide, that's why I was asking whether the different version of the chromedriver 100 vs 106 could be causing the mismatch...

Anyway, you confirmed this is the expected workflow: fair enough, that's it, thank you.

Edit (*) Never mind the -- before the args was wrong! my fault! The following takes effect now:

$ pnpm start -c "browserName=chrome acceptInsecureCerts=true acceptSslCerts=true"  /home/giulio/dso-demo/selenium-ide.side
toddtarsi commented 1 year ago

@giuliohome - Awesome! Glad it's working for you, imo the double hyphens are the worst!

github-actions[bot] commented 1 year ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.