Skip to content

Commit

Permalink
feat: add a reference to the node a rule failed on (#1321)
Browse files Browse the repository at this point in the history
This patch adds a `.node` reference to an errored rule result. This can be used to determine which node the rule errored on and will be helpful for debugging purposes.

Closes #1317.

## Reviewer checks

**Required fields, to be filled out by PR reviewer(s)**
- [x] Follows the commit message policy, appropriate for next version
- [x] Has documentation updated, a DU ticket, or requires no documentation change
- [x] Includes new tests, or was unnecessary
- [x] Code is reviewed for security by: @WilcoFiers
  • Loading branch information
stephenmathieson authored and WilcoFiers committed Jan 21, 2019
1 parent 79e61a0 commit 68741de
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 4 deletions.
5 changes: 4 additions & 1 deletion lib/core/base/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,10 @@ function getDefferedRule(rule, context, options) {
description: 'An error occured while running this rule',
message: err.message,
stack: err.stack,
error: err
error: err,
// Add a serialized reference to the node the rule failed on for easier debugging.
// See https://github.com/dequelabs/axe-core/issues/1317.
errorNode: err.errorNode
});
// resolve
resolve(errResult);
Expand Down
10 changes: 9 additions & 1 deletion lib/core/base/check.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*global CheckResult */
/*global CheckResult,DqElement */

function createExecutionContext(spec) {
/*eslint no-eval:0 */
Expand Down Expand Up @@ -58,6 +58,7 @@ Check.prototype.enabled = true;
* @param {Function} callback Function to fire when check is complete
*/
Check.prototype.run = function(node, options, context, resolve, reject) {
/* eslint max-statements: ["error", 17] */
'use strict';
options = options || {};
var enabled = options.hasOwnProperty('enabled')
Expand All @@ -84,6 +85,13 @@ Check.prototype.run = function(node, options, context, resolve, reject) {
context
);
} catch (e) {
// In the "Audit#run: should run all the rules" test, there is no `node` here. I do
// not know if this is intentional or not, so to be safe, we guard against the
// possible reference error.
if (node && node.actualNode) {
// Save a reference to the node we errored on for futher debugging.
e.errorNode = new DqElement(node.actualNode).toJSON();
}
reject(e);
return;
}
Expand Down
5 changes: 3 additions & 2 deletions test/core/base/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -1035,9 +1035,10 @@ describe('Audit', function() {
throw err;
}
});

axe._tree = axe.utils.getFlattenedTree(fixture);
axe._selectorData = axe.utils.getSelectorData(axe._tree);
a.run(
{ include: [axe.utils.getFlattenedTree(fixture)[0]] },
{ include: [axe._tree[0]] },
{
runOnly: {
type: 'rule',
Expand Down

0 comments on commit 68741de

Please sign in to comment.