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

fix(webdriverio): fix types between v8 and <v8 #962

Merged
merged 6 commits into from
Dec 21, 2023
Merged

fix(webdriverio): fix types between v8 and <v8 #962

merged 6 commits into from
Dec 21, 2023

Conversation

straker
Copy link
Contributor

@straker straker commented Dec 20, 2023

This change brings the types from webdriverio v8 and makes them compatible with the types from <= v7. This change also prepares us to take the latest webdriverio v8 type changes which were causing our builds to break.

This also upgrades the version of @wdio packages in test/wdio in order to not have multiple copies across packages. We should update all these types together so they don't go out-of-sync.

QA notes:

With this change we should be able to run @axe-core/webdriverio package in webdriverio v5+. Using the following code, install each version of webdriverio from v5 to v8, and then compile and run using typescript to verify working

import { AxeBuilder } from '@axe-core/webdriverio'
import { remote } from 'webdriverio'

(async () => {
  const client = await remote({
    logLevel: 'error',
    capabilities: {
      browserName: 'chrome',
      'goog:chromeOptions': {
        args: ['headless', 'disable-gpu']
      }
    }
  })
  await client.url('https://dequeuniversity.com/demo/mars/')

  try {
    const results = await new AxeBuilder({ client }).analyze()
    console.log(results)
  } catch (e) {
    console.error(e)
  }

  client.deleteSession()
})()

Additional notes:

Webdriverio v7 cannot run in node v18/20, so you'll need to use node v16 in order to test <=v7. For webdriverio v5, the code above should compile but may not open a browser and run. This is a set up issue with the webdriverio and not our code so we are OK if the builds but doesn't run in v5.

Closes: #829

@straker straker requested a review from a team as a code owner December 20, 2023 22:20
@@ -59,7 +59,7 @@ jobs:
- run: npm ci
# HACK: Force a TTY to enable browser-driver-manager to manipulate stdout.
- shell: 'script -q -e -c "bash {0}"'
run: npx browser-driver-manager install chromedriver --verbose
run: npx browser-driver-manager install chromedriver=119 --verbose
Copy link
Contributor Author

@straker straker Dec 20, 2023

Choose a reason for hiding this comment

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

This is a temporary fix that will be fixed in a separate pr to fix the cli tests with 120 #963.

@@ -246,15 +253,15 @@ export default class AxeBuilder {
// ensure we fail quickly if an iframe cannot be loaded (instead of waiting
// the default length of 30 seconds)
const { pageLoad } = await this.client.getTimeouts();
this.client.setTimeout({
(this.client as WebdriverIO.Browser).setTimeout({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a problem with older versions of webdriverio as in newer versions they declare the function with the first parameter as this: Webdriverio.Browser.

context?: SerialContextObject,
options?: RunOptions
): Promise<PartialResult> => {
return promisify(
client
.executeAsync<string, []>(
Copy link
Contributor Author

@straker straker Dec 21, 2023

Choose a reason for hiding this comment

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

Older versions of webdriverio don't declare the type of the return so we can't declare it either. This defaults to <unknown, []> for the type in newer versions which is why we need to make the change below to make the return type unknown and then cast it to a string

Copy link
Contributor

@Zidious Zidious left a comment

Choose a reason for hiding this comment

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

Looking good, just a few minor comments inline -

packages/webdriverio/src/utils.ts Outdated Show resolved Hide resolved
packages/webdriverio/src/types.ts Outdated Show resolved Hide resolved
packages/webdriverio/src/utils.ts Outdated Show resolved Hide resolved
straker and others added 2 commits December 21, 2023 09:17
Co-authored-by: Gabe <41127686+Zidious@users.noreply.github.com>
Copy link
Contributor

@Zidious Zidious left a comment

Choose a reason for hiding this comment

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

LGTM!

@straker straker merged commit 0511338 into develop Dec 21, 2023
30 checks passed
@straker straker deleted the fix-wdio branch December 21, 2023 16:27
@github-actions github-actions bot mentioned this pull request Jan 9, 2024
@dequejenn dequejenn mentioned this pull request Jan 15, 2024
@github-actions github-actions bot mentioned this pull request Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

webdriverio: Update to latest webdriverio
2 participants