Skip to content

Commit

Permalink
core: rename webapp-install-banner audit to installable-manifest (#6630)
Browse files Browse the repository at this point in the history
  • Loading branch information
brendankenny authored Nov 27, 2018
1 parent 7715eda commit 3f0a0dd
Show file tree
Hide file tree
Showing 15 changed files with 95 additions and 87 deletions.
4 changes: 2 additions & 2 deletions lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ Object {
"path": "redirects",
},
Object {
"path": "webapp-install-banner",
"path": "installable-manifest",
},
Object {
"path": "splash-screen",
Expand Down Expand Up @@ -840,7 +840,7 @@ Object {
},
Object {
"group": "pwa-installable",
"id": "webapp-install-banner",
"id": "installable-manifest",
"weight": 2,
},
Object {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/test/smokehouse/offline-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ module.exports = {
'user-timings',
'critical-request-chains',
'render-blocking-resources',
'webapp-install-banner',
'installable-manifest',
'splash-screen',
'themed-omnibox',
'aria-valid-attr',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,10 @@ module.exports = [
'critical-request-chains': {
scoreDisplayMode: 'not-applicable',
},
'webapp-install-banner': {
'installable-manifest': {
score: 0,
explanation: 'Failures: No manifest was fetched.',
details: {items: [{isParseFailure: true}]},
},
'splash-screen': {
score: 0,
Expand Down Expand Up @@ -124,7 +126,7 @@ module.exports = [
'critical-request-chains': {
scoreDisplayMode: 'not-applicable',
},
'webapp-install-banner': {
'installable-manifest': {
score: 1,
},
'splash-screen': {
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-cli/test/smokehouse/pwa-expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ module.exports = [
'load-fast-enough-for-pwa': {
// Ignore speed test; just verify that it ran.
},
'webapp-install-banner': {
'installable-manifest': {
score: 1,
details: {items: [pwaDetailsExpectations]},
},
Expand Down Expand Up @@ -110,7 +110,7 @@ module.exports = [
'load-fast-enough-for-pwa': {
// Ignore speed test; just verify that it ran.
},
'webapp-install-banner': {
'installable-manifest': {
score: 1,
details: {items: [pwaDetailsExpectations]},
},
Expand Down
5 changes: 3 additions & 2 deletions lighthouse-cli/test/smokehouse/pwa2-expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ module.exports = [
'load-fast-enough-for-pwa': {
// Ignore speed test; just verify that it ran.
},
'webapp-install-banner': {
'installable-manifest': {
score: 0,
details: {items: [jakeExpectations]},
explanation: /^Failures: .*short_name/,
},
'splash-screen': {
score: 1,
Expand Down Expand Up @@ -104,7 +105,7 @@ module.exports = [
'load-fast-enough-for-pwa': {
// Ignore speed test; just verify that it ran.
},
'webapp-install-banner': {
'installable-manifest': {
score: 1,
details: {items: [pwaDetailsExpectations]},
},
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/test/smokehouse/pwa3-expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ module.exports = [
'load-fast-enough-for-pwa': {
// Ignore speed test; just verify that it ran .
},
'webapp-install-banner': {
'installable-manifest': {
score: 1,
details: {items: [pwaRocksExpectations]},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
*/
'use strict';

const MultiCheckAudit = require('./multi-check-audit');
const MultiCheckAudit = require('./multi-check-audit.js');
const ManifestValues = require('../computed/manifest-values.js');

/**
* @fileoverview
* Audits if a page is configured to prompt users with the webapp install banner.
* Audits if the page's web app manifest qualifies for triggering a beforeinstallprompt event.
* https://github.com/GoogleChrome/lighthouse/issues/23#issuecomment-270453303
*
* Requirements:
Expand All @@ -20,27 +20,21 @@ const ManifestValues = require('../computed/manifest-values.js');
* * manifest has a valid shortname
* * manifest display property is standalone, minimal-ui, or fullscreen
* * manifest contains icon that's a png and size >= 192px
* * SW is registered, and it owns this page and the manifest's start url
* * Site engagement score of 2 or higher
* This audit covers these requirements with the following exceptions:
* * it doesn't consider SW controlling the starturl
* * it doesn't consider the site engagement score (naturally)
*/

class WebappInstallBanner extends MultiCheckAudit {
class InstallableManifest extends MultiCheckAudit {
/**
* @return {LH.Audit.Meta}
*/
static get meta() {
return {
id: 'webapp-install-banner',
title: 'User can be prompted to Install the Web App',
failureTitle: 'User will not be prompted to Install the Web App',
id: 'installable-manifest',
title: 'Web app manifest meets the installability requirements',
failureTitle: 'Web app manifest does not meet the installability requirements',
description: 'Browsers can proactively prompt users to add your app to their homescreen, ' +
'which can lead to higher engagement. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/install-prompt).',
requiredArtifacts: ['URL', 'ServiceWorker', 'Manifest'],
requiredArtifacts: ['URL', 'Manifest'],
};
}

Expand Down Expand Up @@ -86,7 +80,7 @@ class WebappInstallBanner extends MultiCheckAudit {
*/
static async audit_(artifacts, context) {
const manifestValues = await ManifestValues.request(artifacts.Manifest, context);
const manifestFailures = WebappInstallBanner.assessManifest(manifestValues);
const manifestFailures = InstallableManifest.assessManifest(manifestValues);

return {
failures: [
Expand All @@ -97,4 +91,4 @@ class WebappInstallBanner extends MultiCheckAudit {
}
}

module.exports = WebappInstallBanner;
module.exports = InstallableManifest;
1 change: 0 additions & 1 deletion lighthouse-core/audits/multi-check-audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ class MultiCheckAudit extends Audit {
...result,
...result.manifestValues,
manifestValues: undefined,
warnings: undefined,
allChecks: undefined,
};

Expand Down
7 changes: 0 additions & 7 deletions lighthouse-core/computed/manifest-values.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ const PWA_DISPLAY_VALUES = ['minimal-ui', 'fullscreen', 'standalone'];
const SUGGESTED_SHORTNAME_LENGTH = 12;

class ManifestValues {
static get validityIds() {
return ['hasManifest', 'hasParseableManifest'];
}

/** @typedef {(val: NonNullable<LH.Artifacts.Manifest['value']>) => boolean} Validator */

/**
Expand Down Expand Up @@ -86,8 +82,6 @@ class ManifestValues {
*/
static async compute_(manifest) {
// if the manifest isn't there or is invalid json, we report that and bail
let parseFailureReason;

if (manifest === null) {
return {
isParseFailure: true,
Expand Down Expand Up @@ -115,7 +109,6 @@ class ManifestValues {

return {
isParseFailure: false,
parseFailureReason,
allChecks: remainingChecks,
};
}
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ const defaultConfig = {
'user-timings',
'critical-request-chains',
'redirects',
'webapp-install-banner',
'installable-manifest',
'splash-screen',
'themed-omnibox',
'content-width',
Expand Down Expand Up @@ -371,7 +371,7 @@ const defaultConfig = {
// Installable
{id: 'is-on-https', weight: 2, group: 'pwa-installable'},
{id: 'service-worker', weight: 1, group: 'pwa-installable'},
{id: 'webapp-install-banner', weight: 2, group: 'pwa-installable'},
{id: 'installable-manifest', weight: 2, group: 'pwa-installable'},
// PWA Optimized
{id: 'redirects-http', weight: 2, group: 'pwa-optimized'},
{id: 'splash-screen', weight: 1, group: 'pwa-optimized'},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
'use strict';

const WebappInstallBannerAudit = require('../../audits/webapp-install-banner');
const InstallableManifestAudit = require('../../audits/installable-manifest.js');
const assert = require('assert');
const manifestParser = require('../../lib/manifest-parser');

Expand Down Expand Up @@ -37,7 +37,7 @@ describe('PWA: webapp install banner audit', () => {
artifacts.Manifest = null;
const context = generateMockAuditContext();

return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation.includes('No manifest was fetched'), result.explanation);
});
Expand All @@ -47,7 +47,7 @@ describe('PWA: webapp install banner audit', () => {
const artifacts = generateMockArtifacts();
artifacts.Manifest = manifestParser('{,:}', EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
const context = generateMockAuditContext();
return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation.includes('failed to parse as valid JSON'));
});
Expand All @@ -57,7 +57,7 @@ describe('PWA: webapp install banner audit', () => {
const artifacts = generateMockArtifacts();
artifacts.Manifest = manifestParser('{}', EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
const context = generateMockAuditContext();
return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation);
assert.strictEqual(result.details.items[0].failures.length, 4);
Expand All @@ -66,39 +66,43 @@ describe('PWA: webapp install banner audit', () => {

it('passes with complete manifest and SW', () => {
const context = generateMockAuditContext();
return WebappInstallBannerAudit.audit(generateMockArtifacts(), context).then(result => {
return InstallableManifestAudit.audit(generateMockArtifacts(), context).then(result => {
assert.strictEqual(result.rawValue, true, result.explanation);
assert.strictEqual(result.explanation, undefined, result.explanation);
});
});
});

describe('one-off-failures', () => {
/* eslint-disable camelcase */ // because start_url
it('fails when a manifest contains no start_url', () => {
const artifacts = generateMockArtifacts();
artifacts.Manifest.value.start_url.value = undefined;
const context = generateMockAuditContext();

return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation.includes('start_url'), result.explanation);
const failures = result.details.items[0].failures;
assert.strictEqual(failures.length, 1, failures);

const details = result.details.items[0];
assert.strictEqual(details.failures.length, 1, details.failures);
assert.strictEqual(details.hasStartUrl, false);
assert.strictEqual(details.hasShortName, true);
});
});

/* eslint-disable camelcase */ // because short_name
it('fails when a manifest contains no short_name', () => {
const artifacts = generateMockArtifacts();
artifacts.Manifest.value.short_name.value = undefined;
const context = generateMockAuditContext();

return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation.includes('short_name'), result.explanation);
const failures = result.details.items[0].failures;
assert.strictEqual(failures.length, 1, failures);

const details = result.details.items[0];
assert.strictEqual(details.failures.length, 1, details.failures);
assert.strictEqual(details.hasStartUrl, true);
assert.strictEqual(details.hasShortName, false);
});
});

Expand All @@ -107,11 +111,14 @@ describe('PWA: webapp install banner audit', () => {
artifacts.Manifest.value.name.value = undefined;
const context = generateMockAuditContext();

return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation.includes('name'), result.explanation);
const failures = result.details.items[0].failures;
assert.strictEqual(failures.length, 1, failures);

const details = result.details.items[0];
assert.strictEqual(details.failures.length, 1, details.failures);
assert.strictEqual(details.hasStartUrl, true);
assert.strictEqual(details.hasName, false);
});
});

Expand All @@ -120,11 +127,14 @@ describe('PWA: webapp install banner audit', () => {
artifacts.Manifest.value.icons.value = [];
const context = generateMockAuditContext();

return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation.includes('PNG icon'), result.explanation);
const failures = result.details.items[0].failures;
assert.strictEqual(failures.length, 1, failures);

const details = result.details.items[0];
assert.strictEqual(details.failures.length, 1, details.failures);
assert.strictEqual(details.hasStartUrl, true);
assert.strictEqual(details.hasIconsAtLeast192px, false);
});
});
});
Expand All @@ -133,11 +143,14 @@ describe('PWA: webapp install banner audit', () => {
const artifacts = generateMockArtifacts(manifestDirtyJpgSrc);
const context = generateMockAuditContext();

return WebappInstallBannerAudit.audit(artifacts, context).then(result => {
return InstallableManifestAudit.audit(artifacts, context).then(result => {
assert.strictEqual(result.rawValue, false);
assert.ok(result.explanation.includes('PNG icon'), result.explanation);
const failures = result.details.items[0].failures;
assert.strictEqual(failures.length, 1, failures);

const details = result.details.items[0];
assert.strictEqual(details.failures.length, 1, details.failures);
assert.strictEqual(details.hasStartUrl, true);
assert.strictEqual(details.hasIconsAtLeast192px, false);
});
});
});
10 changes: 5 additions & 5 deletions lighthouse-core/test/results/sample_v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -460,9 +460,9 @@
"overallSavingsMs": 0
}
},
"webapp-install-banner": {
"id": "webapp-install-banner",
"title": "User will not be prompted to Install the Web App",
"installable-manifest": {
"id": "installable-manifest",
"title": "Web app manifest does not meet the installability requirements",
"description": "Browsers can proactively prompt users to add your app to their homescreen, which can lead to higher engagement. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/install-prompt).",
"score": 0,
"scoreDisplayMode": "binary",
Expand Down Expand Up @@ -2951,7 +2951,7 @@
"group": "pwa-installable"
},
{
"id": "webapp-install-banner",
"id": "installable-manifest",
"weight": 2,
"group": "pwa-installable"
},
Expand Down Expand Up @@ -3697,7 +3697,7 @@
},
{
"startTime": 0,
"name": "lh:audit:webapp-install-banner",
"name": "lh:audit:installable-manifest",
"duration": 100,
"entryType": "measure"
},
Expand Down
Loading

0 comments on commit 3f0a0dd

Please sign in to comment.