report(a11y): Don't count non-applicable a11y audits toward score #4052
Conversation
robdodson
changed the title
|
I'm not sure if that travis failure is related to my changes. (edit:paulirish) i restarted the travis build. i doubt its related. just some flakyness |
| elements.forEach(function(element) { | ||
| scratch.appendChild(element); | ||
| }); | ||
| const subAudits = scratch.querySelectorAll('.lh-audit-group .lh-audit'); |
There was a problem hiding this comment.
can we just query for .lh-audit and always return that number?
There was a problem hiding this comment.
sure! would you like me to keep the failure check that returns elements.length if subAudits is 0?
There was a problem hiding this comment.
querying for .lh-audit should just work if I'm remember correctly, if they return different results though I'm probably misremembering and you can ignore me :)
| } else { | ||
| groups.failed.push(audit); | ||
| if (audit.score === 100) { |
There was a problem hiding this comment.
nit: let's collapse this with an else if
| @@ -484,6 +529,12 @@ class CategoryRenderer { | |||
| const passedElem = this._renderPassedAuditsSection(passedElements); | |||
| element.appendChild(passedElem); | |||
|
|
|||
| // don't create a not applicable section if there are no not applicables | |||
| if (!notApplicableElements.length) return element; | |||
There was a problem hiding this comment.
seems like we need to move these if (!array.length) checks inside some braces instead of early return
might be a current bug with no manual audits if everything fails :)
| @@ -50,6 +50,7 @@ class ReportGeneratorV2 { | |||
| * @return {number} | |||
| */ | |||
| static arithmeticMean(items) { | |||
| // debugger; | |||
| // "Not Applicable". | ||
| if (result.notApplicable) { | ||
| audit.weight = 0; | ||
| result.informative = true; |
There was a problem hiding this comment.
super nit: can we move this line to be the last of the three? just something about me expects to see score, weight, other...in that order 😆
|
oof, getting those tests in was rough... Maybe I was doing something silly, but it seemed like I needed to manually edit the For instance, this test, as previously written, should actually fail once the manual audits are added because it just looks at the number of audit-groups and assumes it will be a certain size. It might be nicer to add a class to failed audit groups so they can specifically be selected in tests. As it stands, the only way I could differentiate failing audit groups from the others is by selecting for |
|
I tried regenerating the sample data but it broke an unrelated performance test and i'm not entirely sure why. Maybe we can look at it when folks are back in the office. |
|
looks like axe 2.6.1 is out with the notapplicable fix. let me rev and add it to this pr |
robdodson
force-pushed
the
not-applicable-a11y
branch
from
b46f42e
to
eec0f53
Compare
|
@patrickhulce Just rebased against master because the scoring module was changed a bit. I think this is ready to be squashed and merged once travis comes back. |
robdodson
force-pushed
the
not-applicable-a11y
branch
from
eec0f53
to
871a17e
Compare
|
blargh why are these smoke house tests failing all of a sudden? |
This updates the accessibility report so it no longer counts non-applicable tests toward the user's score. Currently lighthouse gives folks an artificial "boost" to their a11y score because aXe was not filtering out tests which matched 0 nodes. That should be fixed in the next release of aXe (dequelabs/axe-core#637).
This PR also updates how the number of passed/failed/notApplicable audits are reported by counting any grouped audits toward the total. Previously it was just counting the number of groups. So even though we have 35 non-manual a11y audits, it looked like we had far fewer. Here's the new look (note the 12 passed, 15 not applicable):