Skip to content

Commit

Permalink
core(main-resource): adjust main resource identification logic (#4475)
Browse files Browse the repository at this point in the history
  • Loading branch information
kdzwinel authored and patrickhulce committed Feb 15, 2018
1 parent 5ea1de0 commit 2f807e0
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 38 deletions.
13 changes: 1 addition & 12 deletions lighthouse-core/gather/computed/main-resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
'use strict';

const ComputedArtifact = require('./computed-artifact');
const HTTP_REDIRECT_CODE_LOW = 300;
const HTTP_REDIRECT_CODE_HIGH = 399;

/**
* @fileoverview This artifact identifies the main resource on the page. Current solution assumes
Expand All @@ -18,15 +16,6 @@ class MainResource extends ComputedArtifact {
return 'MainResource';
}

/**
* @param {WebInspector.NetworkRequest} record
* @return {boolean}
*/
isMainResource(request) {
return request.statusCode < HTTP_REDIRECT_CODE_LOW ||
request.statusCode > HTTP_REDIRECT_CODE_HIGH;
}

/**
* @param {!DevtoolsLog} devtoolsLog
* @param {!ComputedArtifacts} artifacts
Expand All @@ -35,7 +24,7 @@ class MainResource extends ComputedArtifact {
compute_(devtoolsLog, artifacts) {
return artifacts.requestNetworkRecords(devtoolsLog)
.then(requests => {
const mainResource = requests.find(this.isMainResource);
const mainResource = requests.find(request => request.url === artifacts.URL.finalUrl);

if (!mainResource) {
throw new Error('Unable to identify the main resource');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ function replaceChain(chains, networkRecords) {
describe('CriticalRequestChain gatherer: extractChain function', () => {
it('returns correct data for chain from a devtoolsLog', () => {
const computedArtifacts = Runner.instantiateComputedArtifacts();
computedArtifacts.URL = {finalUrl: 'https://en.m.wikipedia.org/wiki/Main_Page'};
const wikiDevtoolsLog = require('../../fixtures/wikipedia-redirect.devtoolslog.json');
const wikiChains = require('../../fixtures/wikipedia-redirect.critical-request-chains.json');

Expand Down
30 changes: 6 additions & 24 deletions lighthouse-core/test/gather/computed/main-resource-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@ describe('MainResource computed artifact', () => {

it('returns an artifact', () => {
const record = {
statusCode: 404,
url: 'https://example.com',
};
const networkRecords = [
{url: 'http://example.com'},
record,
];
computedArtifacts.requestNetworkRecords = _ => Promise.resolve(networkRecords);
computedArtifacts.URL = {finalUrl: 'https://example.com'};

return computedArtifacts.requestMainResource({}).then(output => {
assert.equal(output, record);
Expand All @@ -33,11 +35,10 @@ describe('MainResource computed artifact', () => {

it('thows when main resource can\'t be found', () => {
const networkRecords = [
{
statusCode: 302,
},
{url: 'https://example.com'},
];
computedArtifacts.requestNetworkRecords = _ => Promise.resolve(networkRecords);
computedArtifacts.URL = {finalUrl: 'https://m.example.com'};

return computedArtifacts.requestMainResource({}).then(() => {
assert.ok(false, 'should have thrown');
Expand All @@ -46,28 +47,9 @@ describe('MainResource computed artifact', () => {
});
});

it('should ignore redirects', () => {
const record = {
statusCode: 404,
};
const networkRecords = [
{
statusCode: 301,
},
{
statusCode: 302,
},
record,
];
computedArtifacts.requestNetworkRecords = _ => Promise.resolve(networkRecords);

return computedArtifacts.requestMainResource({}).then(output => {
assert.equal(output, record);
});
});

it('should identify correct main resource in the wikipedia fixture', () => {
const wikiDevtoolsLog = require('../../fixtures/wikipedia-redirect.devtoolslog.json');
computedArtifacts.URL = {finalUrl: 'https://en.m.wikipedia.org/wiki/Main_Page'};

return computedArtifacts.requestMainResource(wikiDevtoolsLog).then(output => {
assert.equal(output.url, 'https://en.m.wikipedia.org/wiki/Main_Page');
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/gather/fake-driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ module.exports = {
return Promise.resolve();
},
gotoURL() {
return Promise.resolve('https://example.com');
return Promise.resolve('https://www.reddit.com/r/nba');
},
beginEmulation() {
return Promise.resolve();
Expand Down
5 changes: 4 additions & 1 deletion lighthouse-core/test/runner-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,9 @@ describe('Runner', () => {
],

artifacts: {
URL: {
finalUrl: 'https://www.reddit.com/r/nba',
},
devtoolsLogs: {
defaultPass: path.join(__dirname, '/fixtures/perflog.json'),
},
Expand Down Expand Up @@ -509,7 +512,7 @@ describe('Runner', () => {
const config = new Config({
passes: [{
passName: 'firstPass',
gatherers: ['viewport-dimensions'],
gatherers: ['url', 'viewport-dimensions'],
}],

audits: [
Expand Down

0 comments on commit 2f807e0

Please sign in to comment.