Skip to content

Commit

Permalink
fix: set preload:true as default (#1281)
Browse files Browse the repository at this point in the history
This PR does the below:

- Set's `preload: true` by default.
- Updates `preload` related tests.
- Updates `preload` API documentation.
- This allows for `experimental` rules like `css-orientation-lock`, to be run with out passing an extra flag from products like extensions.

Closes issue:

## 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
jeeyyy authored and WilcoFiers committed Dec 18, 2018
1 parent eed6589 commit c9731c8
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 48 deletions.
2 changes: 1 addition & 1 deletion doc/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ This example will process all of the "violations", "incomplete", and "inapplicab

###### <a id='preload-configuration-details'></a> Preload Configuration in Options Parameter

The preload attribute in options parameter accepts a `boolean` or an `object` where an array of assets can be specified.
The `preload` attribute (defaults to `true`) in options parameter, accepts a `boolean` or an `object` where an array of assets can be specified.

1. Specifying a `boolean`

Expand Down
9 changes: 7 additions & 2 deletions lib/core/utils/preload.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ function isValidPreloadObject(preload) {
/**
* Returns a boolean which decides if preload is configured
* @param {Object} options run configuration options (or defaults) passed via axe.run
* @return {boolean}
* @return {boolean} defaults to true
*/
axe.utils.shouldPreload = function shouldPreload(options) {
if (!options || !options.preload) {
return false;
return true; // by default `preload` requested assets eg: ['cssom']
}
if (typeof options.preload === 'boolean') {
return options.preload;
Expand All @@ -35,6 +35,11 @@ axe.utils.getPreloadConfig = function getPreloadConfig(options) {
timeout: axe.constants.preloadAssetsTimeout
};

// if no `preload` is configured via `options` - return default config
if (!options.preload) {
return config;
}

// if type is boolean
if (typeof options.preload === 'boolean') {
return config;
Expand Down
23 changes: 13 additions & 10 deletions test/core/base/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,6 @@ describe('Audit', function() {
audit.addRule({
id: 'preload1',
selector: '*'
// this inherently means this rule does not need preload
});
audit.run = function(context, options, resolve, reject) {
var randomRule = this.rules[0];
Expand Down Expand Up @@ -610,8 +609,9 @@ describe('Audit', function() {
// add a rule and check that does not need preload
audit.addRule({
id: 'no-preload',
selector: 'div#div1', // this inherently means this rule does not need preload
any: ['no-preload-check']
selector: 'div#div1',
any: ['no-preload-check'],
preload: false
});
audit.addCheck({
id: 'no-preload-check',
Expand All @@ -628,7 +628,7 @@ describe('Audit', function() {
// add a rule which needs preload
audit.addRule({
id: 'yes-preload',
selector: 'div#div2', // this inherently means this rule does not need preload
selector: 'div#div2',
preload: true
});

Expand Down Expand Up @@ -694,12 +694,13 @@ describe('Audit', function() {
// add a rule and check that does not need preload
audit.addRule({
id: 'no-preload',
selector: 'div#div1' // this inherently means this rule does not need preload
selector: 'div#div1',
preload: false
});
// add a rule which needs preload
audit.addRule({
id: 'yes-preload',
selector: 'div#div2', // this inherently means this rule does not need preload
selector: 'div#div2',
preload: true,
any: ['yes-preload-check']
});
Expand Down Expand Up @@ -767,12 +768,13 @@ describe('Audit', function() {
// add a rule and check that does not need preload
audit.addRule({
id: 'no-preload',
selector: 'div#div1' // this inherently means this rule does not need preload
selector: 'div#div1',
preload: false
});
// add a rule which needs preload
audit.addRule({
id: 'yes-preload',
selector: 'div#div2', // this inherently means this rule does not need preload
selector: 'div#div2',
preload: true,
any: ['yes-preload-check']
});
Expand Down Expand Up @@ -830,12 +832,13 @@ describe('Audit', function() {
// add a rule and check that does not need preload
audit.addRule({
id: 'no-preload',
selector: 'div#div1' // this inherently means this rule does not need preload
selector: 'div#div1',
preload: false
});
// add a rule which needs preload
audit.addRule({
id: 'yes-preload',
selector: 'div#div2', // this inherently means this rule does not need preload
selector: 'div#div2',
preload: true,
any: ['yes-preload-check']
});
Expand Down
6 changes: 4 additions & 2 deletions test/core/utils/preload-cssom.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ describe('axe.utils.preloadCssom unit tests', function() {
var actual = axe.utils.preloadCssom(args);
actual
.then(function(results) {
assert.lengthOf(results[0], 2); // returned from queue, hence the index look up
// returned from queue, hence the index look up
var cssom = results[0];
assert.isAtLeast(cssom.length, 2);
done();
})
.catch(function(error) {
Expand All @@ -61,7 +63,7 @@ describe('axe.utils.preloadCssom unit tests', function() {
.then(function(results) {
// returned from queue, hence the index look up
var cssom = results[0];
assert.lengthOf(cssom, 2);
assert.isAtLeast(cssom.length, 2);
cssom.forEach(function(o) {
assert.hasAllKeys(o, [
'root',
Expand Down
30 changes: 8 additions & 22 deletions test/core/utils/preload.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ describe('axe.utils.preload', function() {
'use strict';

it('should return a queue', function() {
var options = {
preload: true
};
var actual = axe.utils.preload(options);
var actual = axe.utils.preload({});
assert.isObject(actual);
assert.containsAllKeys(actual, ['then', 'defer', 'catch']);
});
Expand Down Expand Up @@ -66,10 +63,7 @@ describe('axe.utils.preload', function() {

describe('axe.utils.shouldPreload', function() {
it('should return true if preload configuration is valid', function() {
var options = {
preload: true
};
var actual = axe.utils.shouldPreload(options);
var actual = axe.utils.shouldPreload({});
assert.isTrue(actual);
});

Expand All @@ -85,28 +79,20 @@ describe('axe.utils.preload', function() {
});

describe('axe.utils.getPreloadConfig', function() {
it('should throw error if preload configuration is invalid', function() {
var actual = function() {
axe.utils.getPreloadConfig({});
};
var expected = Error;
assert.throws(actual, expected);
it('should return default assets if preload configuration is not set', function() {
var actual = axe.utils.getPreloadConfig({}).assets;
var expected = ['cssom'];
assert.deepEqual(actual, expected);
});

it('should return default assets if preload options is set to true', function() {
var options = {
preload: true
};
var actual = axe.utils.getPreloadConfig(options).assets;
var actual = axe.utils.getPreloadConfig({}).assets;
var expected = ['cssom'];
assert.deepEqual(actual, expected);
});

it('should return default timeout value if not configured', function() {
var options = {
preload: true
};
var actual = axe.utils.getPreloadConfig(options).timeout;
var actual = axe.utils.getPreloadConfig({}).timeout;
var expected = 10000;
assert.equal(actual, expected);
});
Expand Down
5 changes: 2 additions & 3 deletions test/integration/full/css-orientation-lock/incomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('css-orientation-lock incomplete test', function() {
type: 'rule',
values: ['css-orientation-lock']
},
preload: false // same effect if preload was not defined
preload: false
},
function(err, res) {
assert.isNull(err);
Expand All @@ -35,8 +35,7 @@ describe('css-orientation-lock incomplete test', function() {
runOnly: {
type: 'rule',
values: ['css-orientation-lock']
},
preload: true
}
},
function(err, res) {
assert.isNull(err);
Expand Down
6 changes: 2 additions & 4 deletions test/integration/full/css-orientation-lock/passes.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ describe('css-orientation-lock passes test', function() {
runOnly: {
type: 'rule',
values: ['css-orientation-lock']
},
preload: true // same effect if preload was not defined
}
},
function(err, res) {
assert.isNull(err);
Expand Down Expand Up @@ -73,8 +72,7 @@ describe('css-orientation-lock passes test', function() {
runOnly: {
type: 'rule',
values: ['css-orientation-lock']
},
preload: true
}
},
function(err, res) {
assert.isNull(err);
Expand Down
6 changes: 2 additions & 4 deletions test/integration/full/css-orientation-lock/violations.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ describe('css-orientation-lock violations test', function() {
runOnly: {
type: 'rule',
values: ['css-orientation-lock']
},
preload: true // same effect if preload was not defined
}
},
function(err, res) {
assert.isNull(err);
Expand Down Expand Up @@ -88,8 +87,7 @@ describe('css-orientation-lock violations test', function() {
runOnly: {
type: 'rule',
values: ['css-orientation-lock']
},
preload: true // same effect if preload was not defined
}
},
function(err, res) {
assert.isNull(err);
Expand Down

0 comments on commit c9731c8

Please sign in to comment.