Skip to content

Commit

Permalink
test: fix flakiness in rerun integration test (#4598)
Browse files Browse the repository at this point in the history
This addresses some test flakiness in `/test/integration/rerun/rerun.js`
which has caused ~4 e2e test failures in CI builds in the last 3 weeks.
It manifests as a timeout because the test uses assertions within
callbacks to `axe.run` without propogating errors to the `done`
callback; this addresses that similarly to other tests using the run
callback pattern, which should cause new errors in this test to report
the actual assertion failure instead of just timing out.

I wasn't able to locally repro the actual failure, but I think a good
guess as to why it's failing flakily is the same issue we saw with other
tests doing deep comparison of axe result objects (testEnvironment
differing between nearby scans due to windowHeight differences). I
addressed that by reusing the test util we added for doing result
comparison.

I didn't want to hardcode knowledge of the testEnvironment thing into
yet another individual test case, so I also refactored the test utility
to move that knowledge into one location with a comment explaining it.

---------

Co-authored-by: dbjorge <dbjorge@users.noreply.github.com>
  • Loading branch information
dbjorge and dbjorge authored Oct 7, 2024
1 parent c42f0a9 commit 154ff6d
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 52 deletions.
25 changes: 15 additions & 10 deletions test/integration/full/rerun/rerun.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,22 @@ describe('rerun axe in the same tick' + window.location.pathname, function () {

// First run
axe.run(options, function (err1, res1) {
assert.isNull(err1);
try {
assert.isNull(err1);

// Second run, on the same tick
axe.run(options, function (err2, res2) {
assert.isNull(err2);

delete res1.timestamp;
delete res2.timestamp;
assert.deepEqual(res1, res2);
done();
});
// Second run, on the same tick
axe.run(options, function (err2, res2) {
try {
assert.isNull(err2);
axe.testUtils.assertResultsDeepEqual(res1, res2);
done();
} catch (e) {
done(e);
}
});
} catch (e) {
done(e);
}
});
});
});
8 changes: 2 additions & 6 deletions test/integration/full/run-partial/after-method.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ describe('run-partial, after-method', function () {
var axeRunPartialResult = results[0];
var axeRunResult = results[1];
assert.lengthOf(axeRunPartialResult.violations, 0);
axe.testUtils.resultsDeepEqual(axeRunPartialResult, axeRunResult, [
'testEnvironment'
]);
axe.testUtils.assertResultsDeepEqual(axeRunPartialResult, axeRunResult);
done();
})
.catch(done);
Expand All @@ -51,9 +49,7 @@ describe('run-partial, after-method', function () {
assert.lengthOf(nodes, 1);
assert.deepEqual(nodes[0].target, ['#frame1', 'h3']);

axe.testUtils.resultsDeepEqual(axeRunPartialResult, axeRunResult, [
'testEnvironment'
]);
axe.testUtils.assertResultsDeepEqual(axeRunPartialResult, axeRunResult);
done();
})
.catch(done);
Expand Down
8 changes: 2 additions & 6 deletions test/integration/full/run-partial/context-size-focusable.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ describe('run-partial, context-size-focusable', function () {
var axeRunPartialResult = results[0];
var axeRunResult = results[1];
assert.lengthOf(axeRunPartialResult.violations, 0);
axe.testUtils.resultsDeepEqual(axeRunPartialResult, axeRunResult, [
'testEnvironment'
]);
axe.testUtils.assertResultsDeepEqual(axeRunPartialResult, axeRunResult);
done();
})
.catch(done);
Expand All @@ -54,9 +52,7 @@ describe('run-partial, context-size-focusable', function () {
assert.deepEqual(nodes[0].target, ['#fail1', 'html']);
assert.deepEqual(nodes[1].target, ['#fail2', 'iframe', 'html']);

axe.testUtils.resultsDeepEqual(axeRunPartialResult, axeRunResult, [
'testEnvironment'
]);
axe.testUtils.assertResultsDeepEqual(axeRunPartialResult, axeRunResult);
done();
})
.catch(done);
Expand Down
8 changes: 2 additions & 6 deletions test/integration/full/run-partial/page-level.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ describe('run-partial, page-level', function () {
var axeRunResult = results[1];
assert.lengthOf(axeRunPartialResult.incomplete, 0);
assert.lengthOf(axeRunPartialResult.passes, 0);
axe.testUtils.resultsDeepEqual(axeRunPartialResult, axeRunResult, [
'testEnvironment'
]);
axe.testUtils.assertResultsDeepEqual(axeRunPartialResult, axeRunResult);
done();
})
.catch(done);
Expand All @@ -60,9 +58,7 @@ describe('run-partial, page-level', function () {
assert.lengthOf(nodes, 1);
assert.deepEqual(nodes[0].target, ['html']);

axe.testUtils.resultsDeepEqual(axeRunPartialResult, axeRunResult, [
'testEnvironment'
]);
axe.testUtils.assertResultsDeepEqual(axeRunPartialResult, axeRunResult);
done();
})
.catch(done);
Expand Down
49 changes: 25 additions & 24 deletions test/testutils.js
Original file line number Diff line number Diff line change
Expand Up @@ -709,28 +709,29 @@ var commons;
}
}

testUtils.resultsDeepEqual = (
obj1,
obj2,
ignoredPaths = [],
testUtils.assertResultsDeepEqual = (
expected,
actual,
ignoredPaths = [
'timestamp',
// testEnvironment is ignored by default because Chrome's UI animations for the
// "an automated test is controlling this browser" notification can cause
// inconsistencies in windowHeight for otherwise-identical scans.
'testEnvironment'
],
keyPath = 'result'
) => {
// timestamp should always be ignored
if (keyPath === 'result') {
ignoredPaths.push('timestamp');
}

const typeObj1 = getType(obj1);
const typeObj2 = getType(obj2);
const typeObj1 = getType(expected);
const typeObj2 = getType(actual);

axe.utils.assert(
typeObj1 === typeObj2,
`Expected type of ${keyPath} to equal ${typeObj1} but got ${typeObj2}`
);

if (typeObj1 === 'object') {
const res1Keys = Object.keys(obj1);
const res2Keys = Object.keys(obj2);
const res1Keys = Object.keys(expected);
const res2Keys = Object.keys(actual);

axe.utils.assert(
res1Keys.length === res2Keys.length &&
Expand All @@ -743,31 +744,31 @@ var commons;
continue;
}

testUtils.resultsDeepEqual(
obj1[key],
obj2[key],
testUtils.assertResultsDeepEqual(
expected[key],
actual[key],
ignoredPaths,
`${keyPath}.${key}`
);
}
} else if (typeObj1 === 'array') {
axe.utils.assert(
obj1.length === obj2.length,
`Expected ${keyPath} to have length of "${obj1.length}" but got "${obj2.length}"`
expected.length === actual.length,
`Expected ${keyPath} to have length of "${expected.length}" but got "${actual.length}"`
);

for (let i = 0; i < obj1.length; i++) {
testUtils.resultsDeepEqual(
obj1[i],
obj2[i],
for (let i = 0; i < expected.length; i++) {
testUtils.assertResultsDeepEqual(
expected[i],
actual[i],
ignoredPaths,
`${keyPath}[${i}]`
);
}
} else {
axe.utils.assert(
obj1 === obj2,
`Expected ${keyPath} to equal "${obj1}" but got "${obj2}"`
expected === actual,
`Expected ${keyPath} to equal "${expected}" but got "${actual}"`
);
}
};
Expand Down

0 comments on commit 154ff6d

Please sign in to comment.