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

Improve automated webdriver setup documentation #11128

Merged
merged 2 commits into from
Sep 13, 2023
Merged

Improve automated webdriver setup documentation #11128

merged 2 commits into from
Sep 13, 2023

Conversation

marcogrcr
Copy link
Contributor

Proposed changes

Improve the automated webdriver setup documentation introduced in 8.14.

While this feature is awesome, I have a test setup environment with no Internet access. I was confused on how to disable this feature since the documentation did not cover this topic in depth. After looking into the source code to figure it out, I decided to pay-it-forward for other users by updating the documentation accordingly.

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

N/A

Reviewers: @webdriverio/project-committers

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 11, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: marcogrcr / name: Marco Gonzalez (163f440)
  • ✅ login: christian-bromann / name: Christian Bromann (7393d93)

@@ -43,24 +43,31 @@ export interface Connection {
/**
* Protocol to use when communicating with the Selenium standalone server (or driver).
*
* If you specify a value other than the default, the webdriver won't be started automatically.
Copy link
Contributor Author

@marcogrcr marcogrcr Sep 11, 2023

Choose a reason for hiding this comment

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

Reference:

export function definesRemoteDriver(options: Pick<Options.WebDriver, 'user' | 'key' | 'protocol' | 'hostname' | 'port' | 'path'>) {
return Boolean(
(options.protocol && options.protocol !== DEFAULT_PROTOCOL) ||
(options.hostname && options.hostname !== DEFAULT_HOSTNAME) ||
Boolean(options.port) ||
(options.path && options.path !== DEFAULT_PATH) ||
Boolean(options.user && options.key)
)
}

and

// - we are not about to run a cloud session
!definesRemoteDriver(options) &&

Applies for all other documentation updates in this file.

* @default 'localhost'
*/
hostname?: string
/**
* Port your WebDriver server is on.
*
* @default 4444
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ref:

/**
* port of automation driver
*/
port: {
type: 'number'
},


:::caution

If you set a driver binary, the browser won't be downloaded automatically. You'll need to have the corresponding browser already installed or specify a binary path as previously shown.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ref:

// - driver options don't define a binary path
!getDriverOptions(cap).binary

and

export function getDriverOptions (caps: Capabilities.Capabilities) {
return (
caps['wdio:chromedriverOptions'] ||
caps['wdio:geckodriverOptions'] ||
caps['wdio:edgedriverOptions'] ||
// Safaridriver does not have any options as it already
// is installed on macOS
{}
)
}

Applies for all other :::caution entries on this file.

Copy link
Member

Choose a reason for hiding this comment

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

This mentioned before already when we document binary for within the common driver options:

binary
Path to a custom driver binary. If set WebdriverIO won't attempt to download a driver but will use the one provided by this path. Make sure the driver is compatible with the browser you are using.

I think repeating this for all browser again isn't ideal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I missed that! OK, I'm going to remove this in favor of promoting that to a :::caution so it gets more prominency.


:::caution

WebdriverIO won't automatically download Microsoft Edge. Only Chrome and Firefox are supported.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ref:

if (isEdge(cap.browserName)) {
// not yet implemented
return Promise.resolve()


WebdriverIO won't automatically download Microsoft Edge. Only Chrome and Firefox are supported.

If you set a browser binary and omit the browser version and driver binary, WebdriverIO will auto-determine the driver version to download based on the automatically detected Microsoft Edge installation. In some environments, the detected browser may be different from the specified binary. Therefore, it's recommended to always specify the browser version when specifying a browser binary and omitting the driver binary.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ref:

if (isEdge(cap.browserName)) {
return setupEdgedriver(cacheDir, cap.browserVersion)

and

export function setupEdgedriver (cacheDir: string, driverVersion?: string) {
return downloadEdgedriver(driverVersion, cacheDir)
}

and

webdriverio-community/node-edgedriver@7f2fc5d5b/src/install.ts:25-30

```

:::info

WebdriverIO won't automatically download Safari driver as it is already installed on macOS.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ref:

export function getDriverOptions (caps: Capabilities.Capabilities) {
return (
caps['wdio:chromedriverOptions'] ||
caps['wdio:geckodriverOptions'] ||
caps['wdio:edgedriverOptions'] ||
// Safaridriver does not have any options as it already
// is installed on macOS
{}
)
}

browserVersion: '116' // or '116.0.5845.96', 'stable', 'dev', 'canary', 'beta'
browserVersion: '116.0.5845.96' // or 'stable', 'latest', 'dev', 'canary', 'beta'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can't use 116. Ref:

const tag = browserName === Browser.CHROME
? caps.browserVersion || ChromeReleaseChannel.STABLE
: caps.browserVersion || 'latest'
const buildId = await resolveBuildId(browserName, platform, tag)
const installOptions: InstallOptions & { unpack?: true } = {
unpack: true,
cacheDir,
platform,
buildId,
browser: browserName,
downloadProgressCallback: (downloadedBytes, totalBytes) => downloadProgressCallback(`${browserName} (${buildId})`, downloadedBytes, totalBytes)
}
const isCombinationAvailable = await canDownload(installOptions)

and running:

await (async () => {
  const { canDownload, detectBrowserPlatform, resolveBuildId } = await import('@puppeteer/browsers')
  const browser = 'chrome'
  const platform = detectBrowserPlatform()

  const results = []
  for (const tag of ['116', '116.0.5845.96', 'stable', 'latest', 'dev', 'canary', 'beta']) {
    const buildId = await resolveBuildId(browser, platform, tag)
    const result = await canDownload({
      browser,
      platform,
      buildId,
    })

    results.push(`${tag}: ${result}`)
  }

  return results
})()

returns

[
  '116: false',
  '116.0.5845.96: true',
  'stable: true',
  'latest: true',
  'dev: true',
  'canary: true',
  'beta: true'
]

Copy link
Member

Choose a reason for hiding this comment

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

This actually works for me:

[
  '116: true',
  '116.0.5845.96: true',
  'stable: true',
  'latest: true',
  'dev: true',
  'canary: true',
  'beta: true'
]

Copy link
Contributor Author

@marcogrcr marcogrcr Sep 11, 2023

Choose a reason for hiding this comment

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

I ran the code using @puppeteer/browsers version 1.4.6 (used here) that's why I got a different result from you.

Running it with @puppeteer/browsers version 1.7.0 (used here) I get the same result as you. Will revert.

That being said, using just the major version with firefox does not work, even with @puppeteer/browsers version 1.7.0:

await (async () => {
  const { canDownload, detectBrowserPlatform, resolveBuildId } = await import('@puppeteer/browsers')
  const browser = 'firefox'
  const platform = detectBrowserPlatform()

  const results = []
  for (const tag of ['119', '119.0a1', 'latest']) {
    const buildId = await resolveBuildId(browser, platform, tag)
    const result = await canDownload({
      browser,
      platform,
      buildId,
    })

    results.push(`${tag}: ${result}`)
  }

  return results
})()

I get:

[
  '119: false',
  '119.0a1: true',
  'latest: true'
]

@@ -43,24 +43,31 @@ export interface Connection {
/**
* Protocol to use when communicating with the Selenium standalone server (or driver).
*
* If you specify a value other than the default, the webdriver won't be started automatically.
Copy link
Member

Choose a reason for hiding this comment

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

I think we should mention this in a separate section here: https://webdriver.io/docs/driverbinaries rather than as part of the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I'll add a bit more information there then.

@@ -103,25 +103,34 @@ Options: *trace* | *debug* | *info* | *warn* | *error* | *silent*
### protocol
Protocol to use when communicating with the Selenium standalone server (or driver).

If you specify a value other than the default, the webdriver won't be started automatically.
Copy link
Member

Choose a reason for hiding this comment

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

same

Type: `Number`<br />
Default: *4444*
Default: `null`
Copy link
Member

Choose a reason for hiding this comment

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

The default value is not null , it is not defined

Suggested change
Default: `null`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was aware. I was just following the standard set by the other parameters:

### queryParams
Query parameters that are propagated to the driver server.
Type: `Object`
Default: `null`

/**
* A key-value store of query parameters to be added to every selenium request
*/
queryParams: {
type: 'object'
},

I will update the documentation to indicate undefined instead.

browserVersion: '116' // or '116.0.5845.96', 'stable', 'dev', 'canary', 'beta'
browserVersion: '116.0.5845.96' // or 'stable', 'latest', 'dev', 'canary', 'beta'
Copy link
Member

Choose a reason for hiding this comment

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

This actually works for me:

[
  '116: true',
  '116.0.5845.96: true',
  'stable: true',
  'latest: true',
  'dev: true',
  'canary: true',
  'beta: true'
]


:::caution

If you set a driver binary, the browser won't be downloaded automatically. You'll need to have the corresponding browser already installed or specify a binary path as previously shown.
Copy link
Member

Choose a reason for hiding this comment

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

This mentioned before already when we document binary for within the common driver options:

binary
Path to a custom driver binary. If set WebdriverIO won't attempt to download a driver but will use the one provided by this path. Make sure the driver is compatible with the browser you are using.

I think repeating this for all browser again isn't ideal.

Type: `Number`<br />
Default: `4444`
Default: `null`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Default: `null`

@@ -13,27 +13,35 @@ The following options are defined when using the [`webdriver`](https://www.npmjs

Protocol to use when communicating with the driver server.

If you specify a value other than the default, the webdriver won't be started automatically.
Copy link
Member

Choose a reason for hiding this comment

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

same as mentioned above

{
capabilities: [
{
browserName: 'chrome', // or 'firefox', 'msedge'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
browserName: 'chrome', // or 'firefox', 'msedge'
browserName: 'chrome', // or 'firefox', 'msedge', 'safari'

},

afterSession() {
cp?.kill();
Copy link
Member

Choose a reason for hiding this comment

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

cp is not defined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, copy/paste from a toy I had and forgot to rename that. That being said, I will be removing this.

}
```

The previous configuration will indicate WebdriverIO to automatically start/stop the specified `chromedriver` binary using an arbitrary unused port. If you need more control over the driver, simply specify a value for [port](configuration#port) and WebdriverIO will skip the automated start/stop. For example:
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if it is valuable to document this. You example works for a single session but it becomes a lot more complex when multiple sessions are involved. Furthermore you example does the same as when using WebdriverIOs built-in setup because Geckodriver is downloaded when you call start so it will fail without network connections too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure if it is valuable to document this.

Agreed, it's very use-case specific, and the ## Manual setup provides enough information on how to get/start the drivers.

Furthermore you example does the same as when using WebdriverIOs built-in setup because Geckodriver is downloaded when you call start so it will fail without network connections too.

Yeah, I missed to specify the customGeckoDriverPath parameter, which unfortunately is broken. In other words, this won't work:

{
  browserName: 'firefox',
  'wdio:geckodriverOptions': {
    binary: '/path/to/geckodriver'
  }
}

That being said, I have created webdriverio-community/node-geckodriver#222 to fix it.

@marcogrcr
Copy link
Contributor Author

Thanks for the review @christian-bromann! I have replied to all your comments and pushed a new revision. Hope this addresses all your feedback.

website/docs/DriverBinaries.md Outdated Show resolved Hide resolved
website/docs/DriverBinaries.md Outdated Show resolved Hide resolved
website/docs/DriverBinaries.md Outdated Show resolved Hide resolved

:::caution

If you specify a browser `binary` but omit the `browserVersion` and its corresponding driver `binary`, WebdriverIO will auto-determine the driver version to download based on the auto-detected browser installation. In some environments, the auto-detected browser version may differ from the specified binary. Therefore, it's recommended to always specify the browser version when specifying a browser binary and omitting the driver binary.
Copy link
Member

Choose a reason for hiding this comment

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

In some environments, the auto-detected browser version may differ from the specified binary.

Can you elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Example 1

Say you have a linux machine with the following:

  • firefox version A installed in /path/to/firefox

Then you use the following wdio configuration:

{
  browserName: 'firefox',
  'moz:firefoxOptions': {
    binary: '/path/to/firefox'
  }
}

This will invoke setupGeckodriver("...", undefined) in wdio:

} else if (isFirefox(cap.browserName)) {
// "latest" works for setting up browser only but not geckodriver
const version = cap.browserVersion === 'latest' ? undefined : cap.browserVersion
return setupGeckodriver(cacheDir, version)

import { download as downloadGeckodriver } from 'geckodriver'

export function setupGeckodriver (cacheDir: string, driverVersion?: string) {
return downloadGeckodriver(driverVersion, cacheDir)
}

Then in geckodriver:

// get latest version of Geckodriver
if (!geckodriverVersion) {
  // ...
}

The version you'll end up getting will be the latest which may not be compatible with firefox version A.

Example 2

Say you have a linux machine with the following:

  • edge version A installed in /usr/bin/microsoft-edge
  • edge version B installed in /path/to/microsoft-edge
  • PATH environment variable set to /usr/bin

Then you use the following wdio configuration:

{
  browserName: 'msedge',
  'ms:edgeOptions': {
    binary: '/path/to/microsoft-edge'
  }
}

This will invoke setupEdgedriver("...", undefined) in wdio:

if (isEdge(cap.browserName)) {
return setupEdgedriver(cacheDir, cap.browserVersion)

import { download as downloadEdgedriver } from 'edgedriver'

export function setupEdgedriver (cacheDir: string, driverVersion?: string) {
return downloadEdgedriver(driverVersion, cacheDir)
}

Then in edgedriver (ref1, ref2):

// install.ts
import findEdgePath from './finder.js'
// ...

export async function download (
  edgeVersion: string = process.env.EDGEDRIVER_VERSION,
  cacheDir: string = process.env.EDGEDRIVER_CACHE_DIR || os.tmpdir()
) {
  // ...
  if (!edgeVersion) {
    const edgePath = findEdgePath()
    log.info(`Trying to detect Microsoft Edge version from binary found at ${edgePath}`)
    edgeVersion = os.platform() === 'win32' ? await getEdgeVersionWin(edgePath) : await getEdgeVersionUnix(edgePath)
    log.info(`Detected Microsoft Edge v${edgeVersion}`)
  }
}

// finder.ts
// Look for edge by using the which command
function linux() {
  // ...
  return findByWhich(
    EDGE_BINARY_NAMES,
    [{ regex: EDGE_REGEX, weight: 51 }]
  )
}

The version you'll up getting will be A, yet you wanted the driver for B.

Copy link
Member

Choose a reason for hiding this comment

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

I see, to summarise this: if you define either a browser or a driver binary but not both, you will likely end up having no matching binaries and your test will fail. Do you think we can formulate it like this somehow? I had to read the sentence a couple of times without really understanding what it means 🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I've reworded the paragraph. Hopefully this does the trick.

@marcogrcr
Copy link
Contributor Author

@christian-bromann OK, I've replied to your comments and published a new revision. Third time's the charm 🤞!

WHAT?

Improve the automated webdriver setup documentation introduced in `8.14`.

WHY?

While this feature is awesome, I have a test setup environment with no
Internet access. I was confused on how to disable this feature since the
documentation did not cover this topic in depth. After looking into the
source code to figure it out, I decided to pay-it-forward for other
users by updating the documentation accordingly.
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.

Awesome, lgtm 🎉

@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 983ed3d into webdriverio:main Sep 13, 2023
6 of 7 checks passed
@christian-bromann
Copy link
Member

Congrats on your first WebdriverIO contribution 🎉 This is awesome stuff!

@marcogrcr
Copy link
Contributor Author

Thanks!Took a bit more tries than I anticipated, but I'm glad we were able to sort them out!

christian-bromann pushed a commit that referenced this pull request Sep 13, 2023
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Docs 📝 PRs that contain changes to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants