chore: add axe-config-generator package #6395
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dbjorge
suggested changes
Feb 1, 2023
There was a problem hiding this comment.
I think it would make more sense to set up the name/contents of the package according to a user's perspective. From a user's perspective, this package is just a wrapper around the config file, and the index.js bundle that builds the config file is an implementation detail of how this package is built, not something they would ever want to run themselves. I'd recommend:
- Remove "generator" from the package name
- We should consider using namespacing rather than a hyphenated prefix, it's more consistent with how most monorepos are laid out; eg,
@accessibility-insights/axe-config
- We should consider using namespacing rather than a hyphenated prefix, it's more consistent with how most monorepos are laid out; eg,
- Make a new
indexthat just exists to export the contents of the JSON file as a const variable, since that's how most users will want to use it in practice - Ensure that only that thin index file and the JSON file end up in
drop/ - Treat the current
indexfile as a build step, rename it accordingly, ensure it doesn't end up indrop/
dbjorge
reviewed
Feb 1, 2023
dbjorge
reviewed
Feb 4, 2023
dbjorge
approved these changes
Feb 4, 2023
5 tasks
dbjorge
added a commit
to microsoft/accessibility-insights-service
that referenced
this pull request
Feb 22, 2023
…pty-table-header rule) (#2402) #### Details Today, Service decides which axe-core rules to use via a `RuleExclusion` mechanism that starts from axe-core's default ruleset an disables rules we don't use in web. This is inconsistent with how accessibility-insights-web defines its ruleset (via a combination of [axe-core's rule tags and explicit overrides](https://github.com/microsoft/accessibility-insights-web/blob/[email protected]/src/scanner/get-rule-inclusions.ts)). The difference in mechanisms led to a mistake during the axe-core 4.6.3 update where the rulesets became inconsistent between web and service. To ensure consistency between web and service, this PR replaces `rule-exclusion.ts` with a new mechanism based on the recently-added `@accessibility-insights/axe-config` package in the web repo. That package builds an axe config blob based on web's `get-rule-inclusions.ts` mechanism. This update bundles a copy of the current output of that package into `axe-configuration.ts` to form the basis of service's axe configuration, and removes `rule-exclusion.ts` entirely as obsolete. Besides keeping the rules in sync, this also means adds a new axe-core config option that was previously overridden by web but not service (`"pingWaitTime": 1000` instead of the default 500ms). I decided to keep the new value from the web config for consistency with web and because both values are still very comfortably within the margin of error set by the page level timeouts used in `page-timeout-config.ts`, even assuming pages with multiple levels of iframe nesting. ##### Motivation * Make it more reliable to keep web and service rules in sync. * Disables the unintentionally-enabled `empty-table-header` best-practice rule ##### Context * Builds on @sfoslund 's work in microsoft/accessibility-insights-web#6395 * Partially implements (but doesn't complete) #571 * I considered doing a fuller implementation that would involve creating a publishing pipeline for the axe-config package, but I didn't want to tie up the fix for the rule inconsistency in service vs web with that process. This is still an incremental improvement that moves us closer to that end state without the NPM package layer. * Incorporates changes from microsoft/accessibility-insights-web#6448 and microsoft/accessibility-insights-web#6449 ##### Testing Besides the PR checklist, I validated that a local build of the scan package against test site https://www.catalog.update.microsoft.com/Home.aspx produced a report consistent with [email protected]'s results (no violations), where previously the service reported a slightly distinct set of errors (one violation of the `empty-table-header` rule, which was accidentally omitted from `rule-exclusions.ts` during the 4.6.3 update). #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [x] Addresses an existing issue: Partially addresses #571 - [x] Added relevant unit test for your changes. (`yarn test`) - [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] Ran precheckin (`yarn precheckin`) - [ ] Validated in an Azure resource group
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Details
Add a
axe-config-generatorpackage, which generates anaxe-config.jsonfile containing the current ScanOptions we provide to axe.run for fastpass scans.Motivation
Keep tool in centralized location
Context
We have been noticing confusion from users about how/ why the set of rules that the web extension uses differs from the set of rules that axe-core provides. This tool is the first step in increasing clarity about which rules the extension is running.
Pull request checklist
yarn fastpassyarn test)<rootDir>/test-results/unit/coveragefix:,chore:,feat(feature-name):,refactor:). SeeCONTRIBUTING.md.