Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(StartUrl): switch from error to debugString object #2549

Merged
merged 10 commits into from
Jul 11, 2017
Merged

refactor(StartUrl): switch from error to debugString object #2549

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fbd724e
refactor(StartUrl): switch from error to debugString object
patrickhulce Jun 20, 2017
d3c8dfd
feedback
patrickhulce Jun 21, 2017
f98d4b9
Merge branch 'master' into start_url_debug_string
patrickhulce Jun 22, 2017
6589cdd
allow debugString during passing
patrickhulce Jun 22, 2017
1cf9b18
fix test
patrickhulce Jun 23, 2017
369058d
pre-existing bug fix
patrickhulce Jun 23, 2017
6cefcd6
Merge branch 'master' into start_url_debug_string
patrickhulce Jun 29, 2017
5c9b84b
Merge branch 'master' into start_url_debug_string
patrickhulce Jul 11, 2017
4a0e7d4
more feedback
patrickhulce Jul 11, 2017
c552cab
more more feedback
patrickhulce Jul 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lighthouse-core/audits/multi-check-audit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,16 @@ class MultiCheckAudit extends Audit {
};
}

let debugString;
if (result.warnings && result.warnings.length > 0) {
debugString = `Warnings: ${result.warnings.join(', ')}`;
}

// Otherwise, we pass
return {
rawValue: true,
extendedInfo,
debugString: result.debugString,
debugString,
};
}

Expand Down
11 changes: 5 additions & 6 deletions lighthouse-core/audits/webapp-install-banner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,20 +79,19 @@ class WebappInstallBanner extends MultiCheckAudit {

if (!hasOfflineStartUrl) {
result.failures.push('Manifest start_url is not cached by a Service Worker');
}

if (artifacts.StartUrl.debugString) {
result.failures.push(artifacts.StartUrl.debugString);
}
} else if (artifacts.StartUrl.debugString) {
result.debugString = artifacts.StartUrl.debugString;
if (artifacts.StartUrl.debugString) {
result.warnings.push(artifacts.StartUrl.debugString);
}
}

static audit_(artifacts) {
const failures = [];
const warnings = [];

return artifacts.requestManifestValues(artifacts.Manifest).then(manifestValues => {
const result = {failures, manifestValues};
const result = {warnings, failures, manifestValues};
WebappInstallBanner.assessManifest(artifacts, result);
WebappInstallBanner.assessServiceWorker(artifacts, result);
WebappInstallBanner.assessOfflineStartUrl(artifacts, result);
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/audits/webapp-install-banner-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ describe('PWA: webapp install banner audit', () => {

return WebappInstallBannerAudit.audit(artifacts).then(result => {
assert.strictEqual(result.rawValue, true);
assert.equal(result.debugString, 'Warning!');
assert.equal(result.debugString, 'Warnings: Warning!');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

});
});
});