Skip to content

CLI update for wdio-electron-service v5#11362

Merged
christian-bromann merged 13 commits intowebdriverio:mainfrom
goosewobbler:update-wizard-electron-service
Oct 9, 2023
Merged

CLI update for wdio-electron-service v5#11362
christian-bromann merged 13 commits intowebdriverio:mainfrom
goosewobbler:update-wizard-electron-service

Conversation

@goosewobbler
Copy link
Contributor

@goosewobbler goosewobbler commented Oct 6, 2023

Proposed changes

wdio-electron-service v5 has recently been released which significantly changed the interface with WDIO. The wdio-cli package no longer produces valid configuration for the service so needs to be updated.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works

Further comments

Electron Forge is explicitly mentioned here but is not supported by the service getBinaryPath util yet. All non-electron-builder cases fall back to asking for the appBinaryPath, as does the case where the electron-builder config file is not JSON.

Reviewers: @webdriverio/project-committers

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 6, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome, thanks!

🙏

@goosewobbler goosewobbler marked this pull request as draft October 6, 2023 18:21
@goosewobbler goosewobbler marked this pull request as ready for review October 7, 2023 18:20
@goosewobbler
Copy link
Contributor Author

@christian-bromann I simplified this a fair bit for the electron-builder case. Basically it just asks for the EB config path and if a JSON file, pulls out the relevant data and passes it to the getBinaryPath util. Other types of EB config file are treated the same as the non-EB cases, e.g. fall back to asking for the electron binary path.

I couldn't find a way to easily test the output in units so just did some manual verification...

@christian-bromann
Copy link
Member

I couldn't find a way to easily test the output in units so just did some manual verification...

I tested myself and looks good to me. There is an open issue for testing the CLI wizard behavior and it is a more complex process.

Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@christian-bromann christian-bromann added the PR: Polish 💅 PRs that contain improvements on existing features label Oct 9, 2023
@christian-bromann christian-bromann merged commit 5f8acd6 into webdriverio:main Oct 9, 2023
appPath?: string
electronAppBinaryPath?: string
electronBuildTool?: ElectronBuildToolChoice
electronBuilderConfigPath: string
Copy link
Contributor

@jan-molak jan-molak Oct 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like electronBuilderConfigPath property should have been marked as optional. I can fix it in #11368 since I'm in the area ;)

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

Labels

PR: Polish 💅 PRs that contain improvements on existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants