Skip to content

Commit 12bfb3a

Browse files
paulirishpaulirish
committed
Manifest tests: Always use the manifest parser
Some tests were testing presence of artifact when they believed to be testing the presence of a property in a manifest.
1 parent 81e98d0 commit 12bfb3a

File tree

10 files changed

+186
-95
lines changed

10 files changed

+186
-95
lines changed

‎lighthouse-core/audits/manifest-background-color.js‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
'use strict';
1919

2020
const Audit = require('./audit');
21+
const Formatter = require('../formatters/formatter');
22+
2123

2224
class ManifestBackgroundColor extends Audit {
2325
/**
@@ -39,19 +41,24 @@ class ManifestBackgroundColor extends Audit {
3941
static hasBackgroundColorValue(manifest) {
4042
return manifest !== undefined &&
4143
manifest.background_color !== undefined &&
42-
manifest.background_color.value !== undefined;
44+
manifest.background_color.value;
4345
}
4446

4547
/**
4648
* @param {!Artifacts} artifacts
4749
* @return {!AuditResult}
4850
*/
4951
static audit(artifacts) {
50-
const hasBackgroundColor = ManifestBackgroundColor
51-
.hasBackgroundColorValue(artifacts.Manifest.value);
52+
const bgColor = ManifestBackgroundColor.hasBackgroundColorValue(artifacts.Manifest.value);
5253

5354
return ManifestBackgroundColor.generateAuditResult({
54-
rawValue: hasBackgroundColor
55+
rawValue: !!bgColor,
56+
extendedInfo: {
57+
value: {
58+
color: bgColor
59+
},
60+
formatter: Formatter.SUPPORTED_FORMATS.NULL
61+
}
5562
});
5663
}
5764
}

‎lighthouse-core/test/audits/background-color.js‎

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,40 +15,60 @@
1515
*/
1616
const Audit = require('../../audits/manifest-background-color.js');
1717
const assert = require('assert');
18+
const manifestSrc = JSON.stringify(require('../fixtures/manifest.json'));
19+
const manifestParser = require('../../lib/manifest-parser');
20+
const exampleManifest = manifestParser(manifestSrc);
21+
1822

1923
/* global describe, it*/
2024

2125
// Need to disable camelcase check for dealing with background_color.
2226
/* eslint-disable camelcase */
2327
describe('Manifest: background color audit', () => {
24-
it('fails when no manifest present', () => {
28+
it('fails when no manifest artifact present', () => {
2529
return assert.equal(Audit.audit({Manifest: {
2630
value: undefined
2731
}}).rawValue, false);
2832
});
2933

30-
it('fails when no background color present', () => {
31-
return assert.equal(Audit.audit({Manifest: {
32-
value: {
33-
foo: 1
34-
}
35-
}}).rawValue, false);
34+
it('fails when an empty manifest is present', () => {
35+
const artifacts = {
36+
Manifest: manifestParser('{}')
37+
};
38+
return assert.equal(Audit.audit(artifacts).rawValue, false);
3639
});
3740

38-
it('fails when no background color value present', () => {
39-
return assert.equal(Audit.audit({Manifest: {
40-
value: {
41+
it('fails when a minimal manifest contains no background_color', () => {
42+
const artifacts = {
43+
Manifest: manifestParser(JSON.stringify({
44+
start_url: '/'
45+
}))
46+
};
47+
return assert.equal(Audit.audit(artifacts).rawValue, false);
48+
});
49+
50+
it('fails when a minimal manifest contains an invalid background_color', () => {
51+
const artifacts = {
52+
Manifest: manifestParser(JSON.stringify({
4153
background_color: 'no'
42-
}
43-
}}).rawValue, false);
54+
}))
55+
};
56+
return assert.equal(Audit.audit(artifacts).rawValue, false);
4457
});
4558

46-
it('passes when color is present', () => {
47-
return assert.equal(Audit.audit({Manifest: {
48-
value: {
49-
background_color: {value: 'black'}
50-
}
51-
}}).rawValue, true);
59+
it('succeeds when a minimal manifest contains a valid background_color', () => {
60+
const artifacts = {
61+
Manifest: manifestParser(JSON.stringify({
62+
background_color: '#FAFAFA'
63+
}))
64+
};
65+
const output = Audit.audit(artifacts);
66+
assert.equal(output.rawValue, true);
67+
assert.equal(output.extendedInfo.value.color, '#FAFAFA');
68+
});
69+
70+
it('succeeds when a complete manifest contains a background_color', () => {
71+
return assert.equal(Audit.audit({Manifest: exampleManifest}).rawValue, true);
5272
});
5373
});
5474
/* eslint-enable */

‎lighthouse-core/test/audits/cache-start-url.js‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const Audit = require('../../audits/cache-start-url.js');
1717
const assert = require('assert');
1818
const manifestSrc = JSON.stringify(require('../fixtures/manifest.json'));
1919
const manifestParser = require('../../lib/manifest-parser');
20-
const Manifest = manifestParser(manifestSrc);
20+
const exampleManifest = manifestParser(manifestSrc);
2121
const CacheContents = ['https://another.example.com/', 'https://example.com/'];
2222
const URL = 'https://example.com';
2323
const AltURL = 'https://example.com/?utm_source=http203';
@@ -30,50 +30,50 @@ describe('Cache: start_url audit', () => {
3030
});
3131

3232
it('fails when no cache contents given', () => {
33-
return assert.equal(Audit.audit({Manifest, URL}).rawValue, false);
33+
return assert.equal(Audit.audit({Manifest: exampleManifest, URL}).rawValue, false);
3434
});
3535

3636
it('fails when no URL given', () => {
37-
return assert.equal(Audit.audit({Manifest, CacheContents}).rawValue, false);
37+
return assert.equal(Audit.audit({Manifest: exampleManifest, CacheContents}).rawValue, false);
3838
});
3939

4040
// Need to disable camelcase check for dealing with start_url.
4141
/* eslint-disable camelcase */
4242
it('fails when a manifest artifact contains no start_url', () => {
43-
const inputs = {
43+
const artifacts = {
4444
Manifest: { }
4545
};
46-
return assert.equal(Audit.audit(inputs).rawValue, false);
46+
return assert.equal(Audit.audit(artifacts).rawValue, false);
4747
});
4848

4949
it('fails when a manifest artifact contains a null start_url', () => {
50-
const inputs = {
50+
const artifacts = {
5151
Manifest: {
5252
start_url: null
5353
}
5454
};
55-
return assert.equal(Audit.audit(inputs).rawValue, false);
55+
return assert.equal(Audit.audit(artifacts).rawValue, false);
5656
});
5757

5858
it('fails when a manifest contains no start_url', () => {
59-
const inputs = {
60-
Manifest: manifestParser({})
59+
const artifacts = {
60+
Manifest: manifestParser('{}')
6161
};
62-
return assert.equal(Audit.audit(inputs).rawValue, false);
62+
return assert.equal(Audit.audit(artifacts).rawValue, false);
6363
});
6464
/* eslint-enable camelcase */
6565

6666
it('succeeds when given a manifest with a start_url, cache contents, and a URL', () => {
6767
return assert.equal(Audit.audit({
68-
Manifest,
68+
Manifest: exampleManifest,
6969
CacheContents,
7070
URL
7171
}).rawValue, true);
7272
});
7373

7474
it('handles URLs with utm params', () => {
7575
return assert.equal(Audit.audit({
76-
Manifest,
76+
Manifest: exampleManifest,
7777
CacheContents,
7878
URL: AltURL
7979
}).rawValue, true);

‎lighthouse-core/test/audits/display.js‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,12 @@
1414
* limitations under the License.
1515
*/
1616
const Audit = require('../../audits/manifest-display.js');
17+
const manifestParser = require('../../lib/manifest-parser');
1718
const assert = require('assert');
1819

20+
const manifestSrc = JSON.stringify(require('../fixtures/manifest.json'));
21+
const exampleManifest = manifestParser(manifestSrc);
22+
1923
/* global describe, it*/
2024

2125
describe('Mobile-friendly: display audit', () => {
@@ -26,28 +30,36 @@ describe('Mobile-friendly: display audit', () => {
2630
assert.equal(Audit.hasRecommendedValue(undefined), false);
2731
});
2832

29-
it('handles the case where there is no display property', () => {
30-
const output = Audit.audit({Manifest: {}});
33+
it('fails when no manifest artifact present', () => {
34+
return assert.equal(Audit.audit({Manifest: {
35+
value: undefined
36+
}}).rawValue, false);
37+
});
38+
39+
it('handles the case where there is no manifest display property', () => {
40+
const artifacts = {
41+
Manifest: manifestParser({})
42+
};
43+
const output = Audit.audit(artifacts);
3144

3245
assert.equal(output.score, false);
3346
assert.equal(output.displayValue, '');
3447
assert.equal(output.rawValue, false);
3548
});
3649

37-
it('audits a manifest\'s display property', () => {
38-
const expected = 'standalone';
39-
const output = Audit.audit({
40-
Manifest: {
41-
value: {
42-
display: {
43-
value: expected
44-
}
45-
}
46-
}
47-
});
48-
50+
it('succeeds when a manifest has a display property', () => {
51+
const artifacts = {
52+
Manifest: manifestParser(JSON.stringify({
53+
display: 'standalone'
54+
}))
55+
};
56+
const output = Audit.audit(artifacts);
4957
assert.equal(output.score, true);
50-
assert.equal(output.displayValue, expected);
58+
assert.equal(output.displayValue, 'standalone');
5159
assert.equal(output.rawValue, true);
5260
});
61+
62+
it('succeeds when a complete manifest contains a display property', () => {
63+
return assert.equal(Audit.audit({Manifest: exampleManifest}).rawValue, true);
64+
});
5365
});

‎lighthouse-core/test/audits/exists.js‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,34 @@
1616
const Audit = require('../../audits/manifest-exists.js');
1717
const assert = require('assert');
1818

19+
const manifestSrc = JSON.stringify(require('../fixtures/manifest.json'));
20+
const manifestParser = require('../../lib/manifest-parser');
21+
const exampleManifest = manifestParser(manifestSrc);
22+
1923
/* global describe, it*/
2024

2125
describe('Manifest: exists audit', () => {
22-
it('fails when no manifest present', () => {
26+
it('fails when no manifest artifact present', () => {
2327
return assert.equal(Audit.audit({Manifest: {
2428
value: undefined
2529
}}).rawValue, false);
2630
});
2731

28-
it('succeeds when a manifest is present', () => {
32+
it('succeeds when a manifest artifact is present', () => {
2933
return assert.equal(Audit.audit({Manifest: {
3034
value: {}
3135
}}).rawValue, true);
3236
});
3337

38+
it('succeeds when a minimal manifest contains a valid background_color', () => {
39+
const artifacts = {
40+
Manifest: manifestParser(JSON.stringify({
41+
name: 'Lighthouse PWA'
42+
}))
43+
};
44+
return assert.equal(Audit.audit(artifacts).rawValue, true);
45+
});
46+
3447
it('correctly passes through debug strings', () => {
3548
const debugString = 'No href found on <link rel="manifest">.';
3649

@@ -41,4 +54,8 @@ describe('Manifest: exists audit', () => {
4154
}
4255
}).debugString, debugString);
4356
});
57+
58+
it('succeeds with a complete manifest', () => {
59+
return assert.equal(Audit.audit({Manifest: exampleManifest}).rawValue, true);
60+
});
4461
});

‎lighthouse-core/test/audits/icons.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const Audit144 = require('../../audits/manifest-icons-min-144.js');
1919
const Audit192 = require('../../audits/manifest-icons-min-192.js');
2020
const assert = require('assert');
2121
const manifestParser = require('../../lib/manifest-parser');
22+
const exampleManifest = JSON.stringify(require('../fixtures/manifest.json'));
2223

2324
/* global describe, it*/
2425

@@ -58,8 +59,7 @@ describe('Manifest: icons audits', () => {
5859

5960
it('succeeds when a manifest contains icons that are large enough', () => {
6061
// stub manifest contains a 192 icon
61-
const manifestSrc = JSON.stringify(require('../fixtures/manifest.json'));
62-
const Manifest = manifestParser(manifestSrc);
62+
const Manifest = manifestParser(exampleManifest);
6363
assert.equal(Audit144.audit({Manifest}).rawValue, true);
6464
assert.equal(Audit192.audit({Manifest}).rawValue, true);
6565
});

‎lighthouse-core/test/audits/name.js‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,32 +17,43 @@ const Audit = require('../../audits/manifest-name.js');
1717
const assert = require('assert');
1818
const manifestSrc = JSON.stringify(require('../fixtures/manifest.json'));
1919
const manifestParser = require('../../lib/manifest-parser');
20-
const manifest = manifestParser(manifestSrc);
20+
const exampleManifest = manifestParser(manifestSrc);
2121

2222
/* global describe, it*/
2323

2424
describe('Manifest: name audit', () => {
25-
it('fails when no manifest present', () => {
25+
it('fails when no manifest artifact present', () => {
2626
return assert.equal(Audit.audit({Manifest: {
2727
value: undefined
2828
}}).rawValue, false);
2929
});
3030

3131
it('fails when an empty manifest is present', () => {
32-
return assert.equal(Audit.audit({Manifest: {}}).rawValue, false);
32+
const artifacts = {
33+
Manifest: manifestParser('{}')
34+
};
35+
return assert.equal(Audit.audit(artifacts).rawValue, false);
3336
});
3437

3538
it('fails when a manifest contains no name', () => {
36-
const inputs = {
37-
Manifest: {
38-
name: null
39-
}
39+
const artifacts = {
40+
Manifest: manifestParser(JSON.stringify({
41+
display: '/'
42+
}))
4043
};
44+
return assert.equal(Audit.audit(artifacts).rawValue, false);
45+
});
4146

42-
return assert.equal(Audit.audit(inputs).rawValue, false);
47+
it('succeeds when a minimal manifest contains a name', () => {
48+
const artifacts = {
49+
Manifest: manifestParser(JSON.stringify({
50+
name: 'Lighthouse PWA'
51+
}))
52+
};
53+
return assert.equal(Audit.audit(artifacts).rawValue, true);
4354
});
4455

45-
it('succeeds when a manifest contains a name', () => {
46-
return assert.equal(Audit.audit({Manifest: manifest}).rawValue, true);
56+
it('succeeds when a complete manifest contains a name', () => {
57+
return assert.equal(Audit.audit({Manifest: exampleManifest}).rawValue, true);
4758
});
4859
});

0 commit comments

Comments
 (0)