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

feat: webdriver7 #150

Merged
merged 25 commits into from Sep 27, 2022
Merged

Conversation

jasonschroeder-sfdc
Copy link
Collaborator

@jasonschroeder-sfdc jasonschroeder-sfdc commented Sep 9, 2022

This change does the following:

This change is needed as:

mohanraj-r
mohanraj-r previously approved these changes Sep 9, 2022
Copy link
Contributor

@mohanraj-r mohanraj-r left a comment

Choose a reason for hiding this comment

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

LGTM

expect(browser.execute('return axe.getRules().length')).toEqual(full.runOnly.values.length);
it('should invoke functions on axe e.g. getRules', async () => {
await loadMinJS();
return expect(browser.execute('return axe.getRules().length')).resolves.toEqual(full.runOnly.values.length);
Copy link
Contributor

Choose a reason for hiding this comment

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

can't we await instead of returning for consistency?

});

it('should run a11y checks using axe', () => {
checkNumViolations('', {}, a11yIssuesCount, 'return (await axe.run()).violations.length;');
return checkNumViolations('', {}, a11yIssuesCount, 'return (await axe.run()).violations.length;');
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above - await instead of return?

Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't the test be async regardless?

@@ -96,6 +96,7 @@ export async function assertAccessible(opts: Partial<WdioOptions> = {}): Promise
* Verify that the currently loaded page in the browser is accessible.
* Throw an error with the accessibility issues found if it is not accessible.
* Synchronous version of {@link assertAccessible}
* @deprecated Please update to using async method.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 The sync vs async WDIO APIs were a mess. Thanks for cleaning it up.

@codecov
Copy link

codecov bot commented Sep 23, 2022

Codecov Report

Merging #150 (c5b5f21) into master (cd2dfd7) will decrease coverage by 0.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #150      +/-   ##
==========================================
- Coverage   97.35%   97.21%   -0.14%     
==========================================
  Files          22       22              
  Lines         340      323      -17     
  Branches       72       66       -6     
==========================================
- Hits          331      314      -17     
- Misses          8        9       +1     
+ Partials        1        0       -1     
Impacted Files Coverage Δ
packages/common/src/index.ts 100.00% <ø> (ø)
packages/test-utils/src/utils.ts 100.00% <ø> (ø)
packages/jest/src/automatic.ts 86.66% <100.00%> (+3.33%) ⬆️
packages/jest/src/setup.ts 91.30% <100.00%> (-4.16%) ⬇️

webdriverio, @wdio/*, and wdio-chromedriver-service to "7"
package.json and yarn.lock
Convert all sync things to async and promises.

Remove references to @wdio/sync in `.depcheckrc` as it's removed now.
Swap @wdio/mocha-framework for @wdio/jasmine-framework
Let typescript compiler (TSC) know about Jasmine
In Webdriverio 7, there are differences between `WebdriverIO.Browser` and
`WebdriverIO.MultiRemoteBrowser` in the functional types and arguments.

For simplicity, we'll only support single browser sessions.
Update/remove dead Jest snapshots
`@sally/common` is a `devDependency` as it's needed for a `__wdio__` test,
but depcheck really wants it as a `dependency`.
The test relies on overwriting/mocking the global `expect`, which is now
difficult to do due to `import { expect } from '@jest/globals'`
Refactor async/await afterEach
Since we are now using `import { expect } from '@jest/globals'`, the
type info just isn't there for our custom matcher.

It will be available when we upgrade to Jest 28:
https://jestjs.io/blog/2022/04/25/jest-28#expect

For now, we disable eslint rule for these tests.
`testPath` is type `string | undefined`, so coerce it to `string` for
the template expression on the next line. (@typescript-eslint/restrict-template-expressions)

Providing an anonymous function for `afterEach` to clarify `this`
scoping (@typescript-eslint/unbound-method)
refreshing yarn.lock
It's not clear if this is a bad test or bad system-under-test (SUT), but
fake timers are involved. Let's test during beta testing, and also
revisit with Jest 28.
Let's keep WDIO up-to-date, now that we are upgraded to 7.x.
Re-arranging circle CI config to enable node16 testing
Copy link

@callenmas14 callenmas14 left a comment

Choose a reason for hiding this comment

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

There's a lot I don't understand here. I'm going by @mohanraj-r 's LGTM comment :) .

@jasonschroeder-sfdc jasonschroeder-sfdc merged commit 70dafc2 into salesforce:master Sep 27, 2022
github-actions bot pushed a commit that referenced this pull request Oct 14, 2022
## [4.0.0-alpha.1](v3.1.0...v4.0.0-alpha.1) (2022-10-14)

### ⚠ BREAKING CHANGES

* `sa11y` will no longer support Node.js 12, which is EOL since 30 April 2022

### Features

* remove support for Node.js 12 ([#116](#116)) ([43ade48](43ade48))
* semantic-release ([18d2728](18d2728))
* tsconfig package for tsconfig.json ([#174](#174)) ([5ff2fd2](5ff2fd2))
* webdriver7 ([#150](#150)) ([70dafc2](70dafc2))
@github-actions
Copy link

🎉 This PR is included in version 4.0.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this pull request Oct 20, 2022
## [4.0.0](v3.1.0...v4.0.0) (2022-10-20)

### ⚠ BREAKING CHANGES

* Explicitly supporting NodeJS 14.x and 16.x
* `sa11y` will no longer support Node.js 12, which is EOL since 30 April 2022

### Features

* remove support for Node.js 12 ([#116](#116)) ([43ade48](43ade48))
* semantic release ([#227](#227)) ([6b7a180](6b7a180))
* tsconfig package for tsconfig.json ([#174](#174)) ([5ff2fd2](5ff2fd2))
* webdriver7 ([#150](#150)) ([70dafc2](70dafc2))

### Bug Fixes

* set package.json node engines ([#228](#228)) ([42de911](42de911))
@github-actions
Copy link

🎉 This PR is included in version 4.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

3 participants