Skip to content

[WIP] test: switch to karma for testing - #1805

Closed
straker wants to merge 9 commits into
developfrom
karma
Closed

[WIP] test: switch to karma for testing#1805
straker wants to merge 9 commits into
developfrom
karma

Conversation

@straker

@straker straker commented Sep 9, 2019

Copy link
Copy Markdown
Contributor

Switching out grunt for running tests to using karma.

NOTE: tests will fail until I swap out npm test to use karma

Reviewer checks

Required fields, to be filled out by PR reviewer(s)

  • Follows the commit message policy, appropriate for next version
  • Code is reviewed for security

@straker
straker requested a review from a team as a code owner September 9, 2019 17:01
Comment thread test/core/base/audit.js
ready(true);
}, 1000);
}
it('should run rules (that do not need preload) and preload assets simultaneously', function(done) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

showing that we can remove all our phantomjs conditionals

Comment thread test/core/index.js
@@ -1,10 +0,0 @@
describe('index', function() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This file wasn't doing anything as src/core/index.js wasn't redefining these things


fixture.innerHTML =
'<div id="t1"><span></span></div><div id="t2"><em></em></div>';
'<div id="t1"><em></em></div><div id="t2"><em></em></div>';

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We no longer have karma output in the test file so these selectors were now unique and the target didn't need a child selector. Updated to force non-unique nodes

actual
.then(function(cssom) {
assert.isAtLeast(cssom.length, 2);
assert.isAtLeast(cssom.length, 1);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We no longer have additional CSS files in test file so there is only the 1 file stylesheet now

Comment thread test/testutils.js
/*eslint indent: 0*/
var testUtils = {};

var fixture = document.createElement('div');

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We don't use the runner.tmpl file anymore so need to create the fixture element

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You could use something like karma fixture. This will help reduce the number of changes that you have to do to make integration tests work.

Comment thread test/core/base/audit.js
it('should run rules (that do not need preload) and preload assets simultaneously', function(done) {
/**
* Note:
* overriding and resolving both check and preload with a delay,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
* overriding and resolving both check and preload with a delay,
* overridden and resolving both check and preload with a delay,

@straker

straker commented Jan 13, 2020

Copy link
Copy Markdown
Contributor Author

Closing for now. Will try again after 4.0

@straker straker closed this Jan 13, 2020
@WilcoFiers
WilcoFiers deleted the karma branch July 17, 2020 09:36
@straker
straker restored the karma branch August 13, 2020 21:19
@WilcoFiers
WilcoFiers deleted the karma branch January 30, 2023 16:08
chutchins25 added a commit that referenced this pull request Jul 17, 2026
## Summary

Adds `aria-actions` to axe-core's known ARIA attributes so it is
recognized as valid, allowed, and prohibited on the roles the spec
prohibits it on — and re-enables the APG `tabs-actions` example that had
been disabled for lack of `aria-actions` support.

Per the [spec draft](w3c/aria#1805),
`aria-actions`:

- **Value type:** ID reference list → `idrefs`
- **Global:** yes (like `aria-describedby`)
- **Empty allowed:** yes — the spec permits `aria-actions=""` (the
deferred-DOM case) → `allowEmpty: true`
- **Prohibited roles:** the name-prohibited roles the spec also
prohibits it on (all axe `prohibitedAttrs` roles except
`none`/`presentation`, which the spec still permits)
- **ElementInternals reflection:** `ariaActionsElements`

## Accessibility-supported rationale

Following the [Impact on
ARIA](https://github.com/dequelabs/axe-core/blob/develop/doc/accessibility-supported.md#impact-on-aria)
decision framework:

1. Supported by all platforms? No — shipped in WebKit and Firefox;
**Chromium pending**.
2. Does its use negatively impact accessibility? **No** — unsupported
browsers simply ignore the attribute (progressive enhancement), and the
spec hard-guards exposure. → **allow.**

> **Note for reviewers:** the ARIA spec change is still [PR
#1805](w3c/aria#1805), not yet merged — this
aligns to the two engines shipping ahead of spec approval. We can patch
the config later in the unlikely event the spec shifts.

## Changes

**Attribute recognition** — `lib/standards/aria-attrs.js`: add the
`aria-actions` entry (`idrefs`, global, `allowEmpty`).

**Prohibited-on-role** — `lib/standards/aria-roles.js`: add
`aria-actions` to `prohibitedAttrs` for `caption`, `code`, `deletion`,
`emphasis`, `insertion`, `mark`, `paragraph`, `strong`, `subscript`,
`superscript`, `suggestion`. Per
[w3c/aria#1805](w3c/aria#1805) these roles
prohibit it; `none`/`presentation` do not, so they are left unchanged.

**APG test re-enable (Closes #4584)** — bump `aria-practices` to latest
`main` (the `tabs-actions` page did not exist at the previously pinned
commit) and remove it from `skippedPages`. axe recognizes the attribute
but not the authoring *pattern*, so `aria-required-children`
(tabs-actions) and `nested-interactive` (listbox-actions) are disabled
per-page pending #5215.

**Review feedback** — update the stale `wai-aria-1.1` `Source:` comment
to the unversioned WAI-ARIA URL; add an `aria-actions=""` pass case
exercising `allowEmpty`.

## Testing

- `get-global-aria-attrs`, `aria-prohibited-attr` (check +
virtual-rule), `aria-valid-attr`, `aria-allowed-attr`,
`aria-valid-attr-value` unit + integration tests ✓
- Full APG suite green (76 passing) ✓
- `npm run build` clean; no auto-generated committed files change

## Follow-ups

- #5215 — teach `aria-required-children` / `nested-interactive` about
the `aria-actions` pattern, then remove the per-page disables in
`apg.spec.js`

Closes #5199
Closes #4584
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.

2 participants