-
Notifications
You must be signed in to change notification settings - Fork 22
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(jest): add jest accessibility matcher #9
Changes from 1 commit
d9f0b26
7a41187
483291c
4eaaf65
6630be3
a91a3e0
51ec35d
e8055fa
54be107
7609b7b
6c7e86d
f8649b2
b5bee0f
c7184e4
1554fdf
ecbfbf9
2a1c1bf
ba71b58
2481a06
37f5f8f
28e628f
3531458
3dbd29a
342f577
a88ef4d
a832256
97677cb
23d4c4c
f54b9ac
eafdc8c
ca3d7a3
618868c
2546f72
7466a6f
9e56fb0
e415437
e1e271a
8ca12bf
c4a4b8a
4054e00
4a16474
8fc2fa6
9ad3ee0
6e3ee18
2f9f9b3
dbc80f1
349a426
2b64f15
c77f356
2b18c4b
56b4893
3889075
eca6850
6eb2a3b
85609d3
36b0087
cbbe288
a6662f6
e4c2717
97249c9
9b1aa3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,5 @@ | ||
{ | ||
"ignorePaths": ["node_modules/**", "package.json"], | ||
"ignoreWords": [ | ||
"assertAccessible", | ||
"doctoc", | ||
"CNCF", | ||
"SPDX", // license header | ||
"tsdoc" | ||
], | ||
"ignoreWords": ["assertAccessible", "doctoc", "CNCF", "integ", "SPDX", "tsdoc"], | ||
"ignoreRegExpList": ["ruleset"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,3 +7,7 @@ | |
|
||
export { toBeAccessible } from './matcher'; | ||
export { adaptA11yConfig, registerA11yMatchers } from './setup'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now the tests are using it - so it needs to be exported I guess? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it ! Fixed. |
||
|
||
export const jestConfig = { | ||
setupFilesAfterEnv: [require.resolve('./setup')], | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
<!-- START doctoc generated TOC please keep comment here to allow auto update --> | ||
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE --> | ||
|
||
|
||
- [`test-integ`](#test-integ) | ||
|
||
<!-- END doctoc generated TOC please keep comment here to allow auto update --> | ||
|
||
# `test-integ` | ||
|
||
Private package for integration testing @sa11y packages |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/* | ||
* Copyright (c) 2020, salesforce.com, inc. | ||
* All rights reserved. | ||
* SPDX-License-Identifier: BSD-3-Clause | ||
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause | ||
*/ | ||
|
||
describe('integration test @sa11y/jest', () => { | ||
it.skip('should have a11y matchers working with setup in jest.config.js', () => { | ||
// TODO(Fix) : Fails with TypeError: expect(...).toBeAccessible is not a function | ||
expect(document).toBeAccessible(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/* | ||
* Copyright (c) 2020, salesforce.com, inc. | ||
* All rights reserved. | ||
* SPDX-License-Identifier: BSD-3-Clause | ||
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause | ||
*/ | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-var-requires | ||
const { jestConfig } = require('@sa11y/jest'); | ||
|
||
module.exports = { | ||
...jestConfig, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
{ | ||
"name": "@sa11y/test-integ", | ||
"version": "0.1.0", | ||
"private": true, | ||
"description": "Private package for integration testing @sa11y packages", | ||
"license": "BSD-3-Clause", | ||
"homepage": "https://github.com/salesforce/sa11y/tree/master/packages/test-integ#readme", | ||
"repository": { | ||
"type": "git", | ||
"url": "git+https://github.com/salesforce/sa11y.git", | ||
"directory": "packages/test-integ" | ||
}, | ||
"devDependencies": { | ||
"@sa11y/jest": "0.1.0" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
{ | ||
"compilerOptions": { "strict": true }, | ||
"include": ["./packages/**/*.ts", "*.js"] | ||
"include": ["*.js", "./packages/**/*.ts", "./packages/**/*.js"] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trevor-bliss In 2b18c4b I am trying to automate setup of the a11y matchers so that they can be setup once in a project instead of every test module. But I am not able to get it to work. Can you please take a look at see what I am missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would simplify the jest config in the integration test package and work backwards from a working solution.
In
packages/test-integ/jest.config.js
, can you have thesetupFilesAfterEnv
option directly that points to a local file that callsexpect.extend
to add a trivial custom matcher?Once you have that then start using the imports/exports until you get back to where you wanted to be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion, let me try that out - thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trevor-bliss Ran into some issues trying the config jest
I have opened an issue with info #11
If there are no other issues with this PR can we merge this while I continue working on the jest config? I can revert 2b18c4b from this PR and add it to another branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah works for me. Approved the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @trevor-bliss
Modified jest setup as per your suggestions in #11
Can you please review