Skip to content

Fix incorrect browser auto-setup doc#11167

Merged
christian-bromann merged 1 commit intowebdriverio:mainfrom
marcogrcr:main
Sep 13, 2023
Merged

Fix incorrect browser auto-setup doc#11167
christian-bromann merged 1 commit intowebdriverio:mainfrom
marcogrcr:main

Conversation

@marcogrcr
Copy link
Contributor

@marcogrcr marcogrcr commented Sep 13, 2023 •
edited
Loading

Proposed changes

Fix an incorrect statement that says browserVersion is used to locate an existing installation of the browser, when in reality it always trigger a download.

When I contributed #11128, I misunderstood the browserVersion logic. This update corrects this misunderstanding.

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
  • I have added necessary documentation (if appropriate)
  • I have added proper type definitions for new commands (if appropriate)

Further comments

Reviewers: @webdriverio/project-committers

@marcogrcr
Fix incorrect browser auto-setup doc
5acb5ea 
WHAT?

Fix an incorrect statement that says `browserVersion` is used to locate
an existing installation of the browser, when in reality instead it
always trigger a download.

WHY?

When I contributed #11128, I misunderstood the `browserVersion` logic.
This update corrects this misunderstanding.

HOW?

Manually reviewed [packages/wdio-utils/src/driver/utils.ts][1]

[1]: https://github.com/webdriverio/webdriverio/blob/v8.16.7/packages/wdio-utils/src/driver/utils.ts
@marcogrcr
Copy link
Contributor Author

marcogrcr commented Sep 13, 2023 •
edited
Loading

@christian-bromann I missed the ! in here 😅:

webdriverio/packages/wdio-utils/src/driver/utils.ts

Lines 117 to 133 in 15f7f22

if (!caps.browserVersion) {
const executablePath = browserName === Browser.CHROME
? await locateChrome().catch(() => undefined)
: await locateFirefox().catch(() => undefined)
const tag = browserName === Browser.CHROME
? getBuildIdByChromePath(executablePath)
: await getBuildIdByFirefoxPath(executablePath)
/**
* verify that we have a valid Chrome/Firefox browser installed
*/
if (tag) {
return {
executablePath,
browserVersion: await resolveBuildId(browserName, platform, tag)
}
}
}

christian-bromann
christian-bromann approved these changes Sep 13, 2023
View reviewed changes
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.

Thanks for updating this 👍

@christian-bromann christian-bromann added the PR: Docs 📝 PRs that contain changes to the documentation label Sep 13, 2023
@christian-bromann christian-bromann merged commit 220dd56 into webdriverio:main Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@christian-bromann christian-bromann christian-bromann approved these changes

Assignees

No one assigned

Labels

PR: Docs 📝 PRs that contain changes to the documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@marcogrcr @christian-bromann