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

Driver Service Fix #168

Merged
merged 112 commits into from
Oct 2, 2023
Merged

Driver Service Fix #168

merged 112 commits into from
Oct 2, 2023

Conversation

goosewobbler
Copy link
Member

@goosewobbler goosewobbler commented Aug 26, 2023

Updating the service for the breaking changes released with WDIO v8.14.0:

  • Removed dependency on wdio-chromedriver-service
  • WDIO now handles Chromedriver download and processes
  • Improved capabilities mapping, parity with other services
  • Parallel Multiremote supported
  • Service options moved into capabilities
  • Updated all dependencies
  • Improved debugging using debug
  • Replaced appPath / appName / binaryPath with appBinaryPath to reduce complexity
  • New getBinaryPath helper from wdio-electron-service/utils to assist users of appPath / appName
  • Restructured examples
    • app used by ESM example is now also ESM - using Electron 28 (currently nightly)
    • using Rollup instead of Webpack
  • CJS build now using symlinks instead of files with a single require()

TODO:

  • Extract & rework capabilities mapping
    • Ensure the service works with parallel multiremote released in WDIO v8.15.0
    • Fix type handling
    • Add / update unit tests
  • Use custom electron to chromium version map based on electron-to-chromium for delegating CD download to WDIO
    • Add / update unit tests
  • Fix Linux & Mac integration b0rkage
  • Fix WDIO CD spawn errors
  • Fix WDIO CD download flakiness
  • Remove service-managed CD download
  • Remove chromedriverCustomPath in favour of user setting wdio:chromedriverOptions directly
  • Move service options into capabilities
  • Update unit tests
  • Fix linting
  • Update docs

@goosewobbler
Copy link
Member Author

goosewobbler commented Aug 26, 2023

@christian-bromann So, @electron/get is the way we were downloading Chromedriver, I am not sure we can use it to get the CD version string in order to pass it to WDIO; @electron/get is designed to abstract this process and download the appropriate assets for a given electron version. Since WDIO itself is now handling the download, perhaps the best way forward here is to amend the WDIO download logic to use @electron/get when an electronVersion (as passed by this service) value is provided.

@christian-bromann
Copy link
Contributor

I think there are two approaches we can go:

  1. keep the launcher and use @electron/get to download Chromedriver - then in the launcher set the following capabilities:
    'wdio:chromedriverOptions': { binary: '/path/to/chromedriver' },
    'goog:chromeOptions: { binary: '/path/to/electron/app' }
  2. get the Chromedriver version based on the Electron version and update the capabilities as part of the launcher so that WebdriverIOs driver manager handles the download

Wdyt?

@goosewobbler
Copy link
Member Author

goosewobbler commented Aug 27, 2023

@christian-bromann I prototyped the first approach and it's not able to connect to Chromedriver when specifying the hostname / port. Is there anything obvious I could have missed? Output here.

@christian-bromann
Copy link
Contributor

@goosewobbler the port 9519 doesn't seem to be correct, can you remove it from the configuration? WebdriverIO will find a port for you.

@goosewobbler
Copy link
Member Author

goosewobbler commented Aug 28, 2023

Hmmm, that didn't work, it's now trying to connect to http://localhost:undefined/. Removing the hostname as well does run the app but the integration tests stall.

@christian-bromann
Copy link
Contributor

christian-bromann commented Aug 28, 2023

Can you check that you don't set any of these options in the service: hostname, port, path or protocol? I see hostname is still set which makes WebdriverIO to connect to what it sees in the options.

@christian-bromann
Copy link
Contributor

It seems to be working and connecting properly now.

@christian-bromann
Copy link
Contributor

@goosewobbler let me know if there is anything left to do to land this. Amazing work!

@goosewobbler
Copy link
Member Author

goosewobbler commented Oct 2, 2023

@christian-bromann Thanks! With that latest PR I think it's done. Will run a couple of smoke tests with an external repo before release as there are a lot of changes here...

@christian-bromann
Copy link
Contributor

Awesome, please feel free to merge and release as major change anytime you are ready.

@goosewobbler goosewobbler merged commit fbe0241 into main Oct 2, 2023
@goosewobbler goosewobbler deleted the driver-service-fix branch October 2, 2023 21:12
@christian-bromann
Copy link
Contributor

Awesome work 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants