feat(assert): add assert accessible api #7

mohanraj-r · 2020-03-31T22:04:24Z

assert: add assert package with assertAccessible API (0726678)
- This would be paired with a jest matcher (TBD)
format: add basic a11y results format package (b28dc2a)
- This would be fleshed out in a future PR
rules: fix import paths of modules in rules package (fc54a76)

also update cspell ignored word list

yarn upgrade

tests now pass using the wrapper

to not expose implementation details unnecessarily

trevor-bliss · 2020-04-02T05:51:14Z

packages/assert/__tests__/assert.test.ts

+        // TODO: Rename getA11yConfig() to getA11yConfig
+        const errConfig = getA11yConfig(['non-existent-rule']);
+        await assertAccessible(document, errConfig).catch((e) => {
+            expect(e).toBeDefined();


If assertAccessible does not throw an error to enter the catch block the test will still pass.

There are many ways to solve this. One simple way is to declare the number of assertions you expect in each test case to guarantee they are run: https://jestjs.io/docs/en/expect#expectassertionsnumber

Thanks! I was wondering about this too .. Fixed in all tests now.
Fixed the names too. Thanks for the tip 👍

trevor-bliss · 2020-04-02T05:57:42Z

packages/assert/__tests__/assert.test.ts

+                                        </script>
+                                    </body>
+                                    </html>`;
+        await assertAccessible(document, jsdomRules).catch((error) => expect(error).toBeUndefined());


This flow feels weird to me. To me it would make more sense to not have the catch block and let the error thrown bubble up and fail the test case.

Makes sense. Fixed.

trevor-bliss · 2020-04-02T05:58:40Z

packages/assert/__tests__/assert.test.ts

+                                    </body>
+                                   </html>`;
+        await assertAccessible(document, jsdomRules).catch((e) => {
+            expect(e).toBeDefined();


Same issue as above. You should guarantee these assertions are actually run.

trevor-bliss · 2020-04-02T05:59:53Z

package.json

@@ -62,7 +69,8 @@
        "jest": "^25.1.0",
        "lerna": "^3.20.2",
        "lint-staged": "^10.0.8",
-        "prettier": "1.19.1",
+        "lockfile-lint": "^4.2.2",


Nice. I didn't know this existed.

Thanks, yes this is helpful esp since yarn doesn't seem to respect project npmrc when installing deps in packages (yarnpkg/yarn#4458)

Btw looking for a tool that can help ensure the same version of dependency is used across all the packages in the project (something like https://www.npmjs.com/package/syncpack). If you have any recommendations, please let me know.

trevor-bliss · 2020-04-02T06:02:06Z

packages/assert/__tests__/assert.test.ts

+        });
+    });
+
+    it('dom with no a11y issues', async () => {


These test names should be more explicit. These are JUnit style names 😛. Something like "does not throw error when no a11y issues in DOM" would be better. Then when you read the describe + it block it fully describes what exactly you are testing.

Makes sense 👍 Updated.

trevor-bliss · 2020-04-02T06:02:14Z

packages/assert/__tests__/assert.test.ts

+        await assertAccessible(document, jsdomRules).catch((error) => expect(error).toBeUndefined());
+    });
+
+    it('dom with a11y issues', async () => {


Better name as above.

rename test names to be more descriptive

mohanraj-r

Thanks for the review @trevor-bliss ! Appreciate the comments.
Took care of them. Please review.

mohanraj-r · 2020-04-02T23:32:23Z

package.json

@@ -62,7 +69,8 @@
        "jest": "^25.1.0",
        "lerna": "^3.20.2",
        "lint-staged": "^10.0.8",
-        "prettier": "1.19.1",
+        "lockfile-lint": "^4.2.2",


Thanks, yes this is helpful esp since yarn doesn't seem to respect project npmrc when installing deps in packages (yarnpkg/yarn#4458)

Btw looking for a tool that can help ensure the same version of dependency is used across all the packages in the project (something like https://www.npmjs.com/package/syncpack). If you have any recommendations, please let me know.

mohanraj-r · 2020-04-02T23:34:00Z

packages/assert/__tests__/assert.test.ts

+        // TODO: Rename getA11yConfig() to getA11yConfig
+        const errConfig = getA11yConfig(['non-existent-rule']);
+        await assertAccessible(document, errConfig).catch((e) => {
+            expect(e).toBeDefined();


Thanks! I was wondering about this too .. Fixed in all tests now.
Fixed the names too. Thanks for the tip 👍

mohanraj-r · 2020-04-03T04:45:25Z

packages/assert/src/assert.ts

+ * @param formatter - Function to format a11y violations
+ * @throws error - with the accessibility issues found
+ * */
+export async function assertAccessible(


Hi @cordeliadillon Can you please review the assert API. I would like to merge this by EOD tomorrow.

Looking at it now! Thanks for the nudge. :)

cordeliadillon · 2020-04-03T17:07:50Z

packages/assert/README.md

+import { assertAccessible } from '@sa11y/assert';
+
+// Setup DOM ..
+await assertAccessible();


Who's the audience for this readme file? Should this have more detail around assertAccessible's params, what types of errors it throws, and how it can be used in context?

Good point, Updated readme.
Added a TODO to look into generating usage docs from the code comments - as right now its duplicating doc from code and it needs to be kept in sync with code etc.

cordeliadillon · 2020-04-03T17:11:32Z

packages/format/src/format.ts

+ * @param violations - Result list from axe
+ * */
+// TODO: Add handlebars template for formatting
+// TODO: Add support for different output formats console(colored), plain text, HTML, xUnit


cordeliadillon · 2020-04-03T17:17:33Z

packages/preset-rules/src/a11yConfig.ts

@@ -8,9 +8,9 @@
 import { RunOptions } from 'axe-core';

 /**
- * AxeConfig is limited to subset of options that we need and use in this library
+ * A11yConfig contains options to trigger a accessibility run, specifying list of rules to test


"trigger a accessibility run" seems vague. By "accessibility run," do you mean "run of AXE accessibility checks"?

Good point, updated doc.

to check if that fixes the code cov action failure in CI

to a different fork: ziishaned/jest-reporter-action#3

due to issues with current code cov action

github-actions · 2020-04-03T20:46:38Z

Total Coverage: 100.00%

Coverage Report

File	Branches	Funcs	Lines
packages/assert/src
assert.ts	100%	100%	100%
packages/format/src
format.ts	100%	100%	100%
packages/preset-rules/src
a11yConfig.ts	100%	100%	100%
extended.ts	100%	100%	100%
recommended.ts	100%	100%	100%

mohanraj-r · 2020-04-03T20:48:01Z

Looks like the CI setup has some issues. Debugging it now. Will post here once it is fixed.

All CI issues are fixed now.
@cordeliadillon Can you please review and approve.

github-actions · 2020-04-07T02:08:48Z

Coverage after merging feat/add_assert_accessible_api into master

100.00%

Coverage Report

File	Branches	Funcs	Lines
packages/assert/src
assert.ts	100%	100%	100%
packages/format/src
format.ts	100%	100%	100%
packages/preset-rules/src
a11yConfig.ts	100%	100%	100%
extended.ts	100%	100%	100%
index.ts	100%	100%	100%
recommended.ts	100%	100%	100%

Fix changelog script to append rather then regen

github-actions · 2020-04-07T03:21:28Z

Coverage after merging feat/add_assert_accessible_api into master

100.00%

Coverage Report

File	Branches	Funcs	Lines
packages/assert/src
assert.ts	100%	100%	100%
packages/format/src
format.ts	100%	100%	100%
packages/preset-rules/src
a11yConfig.ts	100%	100%	100%
extended.ts	100%	100%	100%
index.ts	100%	100%	100%
recommended.ts	100%	100%	100%

fix test snapshot caused by change in output: header -> heading; dequelabs/axe-core@8890063

github-actions · 2020-04-07T19:38:34Z

Coverage after merging feat/add_assert_accessible_api into master

100.00%

Coverage Report

File	Branches	Funcs	Lines
packages/assert/src
assert.ts	100%	100%	100%
packages/format/src
format.ts	100%	100%	100%
packages/preset-rules/src
a11yConfig.ts	100%	100%	100%
extended.ts	100%	100%	100%
index.ts	100%	100%	100%
recommended.ts	100%	100%	100%

and update packages

github-actions · 2020-04-07T19:49:09Z

Coverage after merging feat/add_assert_accessible_api into master

100.00%

Coverage Report

File	Branches	Funcs	Lines
packages/assert/src
assert.ts	100%	100%	100%
packages/format/src
format.ts	100%	100%	100%
packages/preset-rules/src
a11yConfig.ts	100%	100%	100%
extended.ts	100%	100%	100%
index.ts	100%	100%	100%
recommended.ts	100%	100%	100%

to include bug fixes, vulnerability fixes in recent releases

github-actions · 2020-04-07T23:07:53Z

Coverage after merging feat/add_assert_accessible_api into master

100.00%

Coverage Report

File	Branches	Funcs	Lines
packages/assert/src
assert.ts	100%	100%	100%
packages/format/src
format.ts	100%	100%	100%
packages/preset-rules/src
a11yConfig.ts	100%	100%	100%
extended.ts	100%	100%	100%
index.ts	100%	100%	100%
recommended.ts	100%	100%	100%

cordeliadillon

👍

Mohan Raj Rajamanickam added 17 commits March 25, 2020 13:15

build(root): create a specific tsconfig for eslint to fix lint-staged

76cf944

also update cspell ignored word list

build(root): upgrade deps

51fb083

feat(assert): add scaffolding for assert package

0726678

chore(root): upgrade dependencies

78887ea

chore(root): add explicit deps to packages, type deps, misc format

131b919

build(root): add lint to check yarn lock files for public repos only

0427e16

yarn upgrade

improvement(assert): add WIP assert api, tests

674bf25

improvement(assert): add a promise wrapper to run axe

66f909b

tests now pass using the wrapper

refactor(preset-rules): rename axe config to a11y config

46bb085

to not expose implementation details unnecessarily

improvement(assert): fix/refactor async assert accessible api with await

a876f64

build(root): change code cov config, clean scripts

9a53750

improvement(add result formatter func scaffolding): assert

635bb3b

improvement(format): improve error report formatting

c7a6dc0

feat(format): add basic scaffolding for format package

b28dc2a

refactor(format): move formatter func to its own package

34c44a6

docs(root): add links to package docs, add/fix docs

670cf6e

docs(format): misc doc fixes/updates

eb4c2a4

mohanraj-r marked this pull request as ready for review April 1, 2020 00:07

mohanraj-r requested a review from a team April 1, 2020 00:07

mohanraj-r self-assigned this Apr 1, 2020

mohanraj-r changed the title ~~Feat/add assert accessible api~~ feat(assert): add assert accessible api Apr 1, 2020

test(format): add basic snapshot test for results formatter

3e36d29

trevor-bliss reviewed Apr 2, 2020

View reviewed changes

Mohan Raj Rajamanickam added 2 commits April 2, 2020 15:47

test(assert): add explicit assertion expectations for async tests

7b9e2ef

rename test names to be more descriptive

improvement(format): add option to specify formatter

2948fa0

mohanraj-r commented Apr 2, 2020

View reviewed changes

trevor-bliss previously approved these changes Apr 3, 2020

View reviewed changes

mohanraj-r commented Apr 3, 2020

View reviewed changes

cordeliadillon reviewed Apr 3, 2020

View reviewed changes

docs(root): revise usage docs, misc doc fixes

ef24d37

Mohan Raj Rajamanickam added 6 commits April 3, 2020 12:49

ci(root): add creds, command to code cov action

105b47a

ci(root): fix indendation in CI config yaml file

f59e37f

ci(root): fix CI yaml config syntax error, add CI badge

569ecd9

ci(root): remove custom reporters in code cov config

88e107f

to check if that fixes the code cov action failure in CI

ci(root): switch code cov reporter github CI action

48ba4e5

to a different fork: ziishaned/jest-reporter-action#3

ci(root): switch to a different code cov github action

a27a379

due to issues with current code cov action

ci(root): update to latest release of code cov github action

4035308

This comment has been minimized.

Sign in to view

ci(root): update name of CI workflow, badge in readme

4300afd

This comment has been minimized.

Sign in to view

mohanraj-r requested a review from cordeliadillon April 6, 2020 22:18

fix(rules): fix import paths of modules in rules package

fc54a76

docs(root): update changelog

2a2ad56

Fix changelog script to append rather then regen

Mohan Raj Rajamanickam added 4 commits April 7, 2020 12:24

build(root): update axe-core, fix axe-core version to be exact

a15dd81

fix test snapshot caused by change in output: header -> heading; dequelabs/axe-core@8890063

build(root): update deps

d8f422d

build(root): add lockfile lint check to lint staged

1408f6c

build(root): update deps

a2e2da8

build(root): add yarn audit check to update script

7397f7c

and update packages

build(root): update global-jsdom, commitzen

ba68d8d

to include bug fixes, vulnerability fixes in recent releases

cordeliadillon approved these changes Apr 8, 2020

View reviewed changes

mohanraj-r merged commit 1294f76 into master Apr 8, 2020

mohanraj-r deleted the feat/add_assert_accessible_api branch April 8, 2020 21:15

mohanraj-r mentioned this pull request Apr 8, 2020

Fix build failure in code coverage step on merge to master #8

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(assert): add assert accessible api #7

feat(assert): add assert accessible api #7

mohanraj-r commented Mar 31, 2020 •

edited

Loading

trevor-bliss Apr 2, 2020

mohanraj-r Apr 2, 2020

trevor-bliss Apr 2, 2020

mohanraj-r Apr 2, 2020

trevor-bliss Apr 2, 2020

mohanraj-r Apr 2, 2020

trevor-bliss Apr 2, 2020

mohanraj-r Apr 2, 2020

trevor-bliss Apr 2, 2020

mohanraj-r Apr 2, 2020

trevor-bliss Apr 2, 2020

mohanraj-r Apr 2, 2020

mohanraj-r left a comment

mohanraj-r Apr 2, 2020

mohanraj-r Apr 2, 2020

mohanraj-r Apr 3, 2020

cordeliadillon Apr 3, 2020

cordeliadillon Apr 3, 2020

mohanraj-r Apr 3, 2020

cordeliadillon Apr 3, 2020

cordeliadillon Apr 3, 2020

mohanraj-r Apr 3, 2020

github-actions bot commented Apr 3, 2020

mohanraj-r commented Apr 3, 2020

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Apr 7, 2020

github-actions bot commented Apr 7, 2020

github-actions bot commented Apr 7, 2020

github-actions bot commented Apr 7, 2020

github-actions bot commented Apr 7, 2020

cordeliadillon left a comment

feat(assert): add assert accessible api #7

feat(assert): add assert accessible api #7

Conversation

mohanraj-r commented Mar 31, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mohanraj-r left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 3, 2020

mohanraj-r commented Apr 3, 2020

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Apr 7, 2020

github-actions bot commented Apr 7, 2020

github-actions bot commented Apr 7, 2020

github-actions bot commented Apr 7, 2020

github-actions bot commented Apr 7, 2020

cordeliadillon left a comment

Choose a reason for hiding this comment

mohanraj-r commented Mar 31, 2020 •

edited

Loading