Skip to content

Commit

Permalink
handle old and new trace object format
Browse files Browse the repository at this point in the history
  • Loading branch information
brendankenny committed Aug 4, 2016
1 parent 6663f6b commit 7c9c44f
Show file tree
Hide file tree
Showing 20 changed files with 84 additions and 74 deletions.
2 changes: 1 addition & 1 deletion lighthouse-core/audits/estimated-input-latency.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class EstimatedInputLatency extends Audit {
const startTime = artifacts.Speedline.first;

const trace = artifacts.traces[this.DEFAULT_TRACE] &&
artifacts.traces[this.DEFAULT_TRACE].traceContents;
artifacts.traces[this.DEFAULT_TRACE].traceEvents;
const tracingProcessor = new TracingProcessor();
const model = tracingProcessor.init(trace);
const latencyPercentiles = TracingProcessor.getRiskToResponsiveness(model, trace, startTime);
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/audits/first-meaningful-paint.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class FirstMeaningfulPaint extends Audit {
*/
static audit(artifacts) {
return new Promise((resolve, reject) => {
const traceContents = artifacts.traces[this.DEFAULT_TRACE].traceContents;
const traceContents = artifacts.traces[this.DEFAULT_TRACE].traceEvents;
if (!traceContents || !Array.isArray(traceContents)) {
throw new Error(FAILURE_MESSAGE);
}
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/audits/time-to-interactive.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class TTIMetric extends Audit {

// Process the trace
const tracingProcessor = new TracingProcessor();
const traceContents = artifacts.traces[Audit.DEFAULT_TRACE].traceContents;
const traceContents = artifacts.traces[Audit.DEFAULT_TRACE].traceEvents;
const model = tracingProcessor.init(traceContents);
const endOfTraceTime = model.bounds.max;

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/audits/user-timings.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ class UserTimings extends Audit {
return new Promise((resolve, reject) => {
const traceContents =
artifacts.traces[this.DEFAULT_TRACE] &&
artifacts.traces[this.DEFAULT_TRACE].traceContents;
artifacts.traces[this.DEFAULT_TRACE].traceEvents;
if (!traceContents || !Array.isArray(traceContents)) {
throw new Error(FAILURE_MESSAGE);
}
Expand Down
32 changes: 19 additions & 13 deletions lighthouse-core/config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const log = require('../lib/log');
// and to change TracingStartedInBrowser events into TracingStartedInPage.
// This is done by searching for most occuring threads and basing new events
// off of those.
function cleanTrace(traceContents) {
function cleanTrace(traceEvents) {
// Keep track of most occuring threads
const threads = [];
const countsByThread = {};
Expand All @@ -55,7 +55,7 @@ function cleanTrace(traceContents) {
let name;
let counter;

traceContents.forEach((evt, idx) => {
traceEvents.forEach((evt, idx) => {
if (evt.name.startsWith('TracingStartedIn')) {
traceStartEvents.push(idx);
}
Expand Down Expand Up @@ -90,20 +90,20 @@ function cleanTrace(traceContents) {

// Remove all current TracingStartedIn* events, storing
// the first events ts.
const ts = traceContents[traceStartEvents[0]] && traceContents[traceStartEvents[0]].ts;
const ts = traceEvents[traceStartEvents[0]] && traceEvents[traceStartEvents[0]].ts;

// account for offset after removing items
let i = 0;
for (let dup of traceStartEvents) {
traceContents.splice(dup - i, 1);
traceEvents.splice(dup - i, 1);
i++;
}

// Add a new TracingStartedInPage event based on most active thread
// and using TS of first found TracingStartedIn* event
traceContents.unshift(makeMockEvent(mostActiveFrame, ts));
traceEvents.unshift(makeMockEvent(mostActiveFrame, ts));

return traceContents;
return traceEvents;
}

function filterPasses(passes, audits) {
Expand Down Expand Up @@ -185,20 +185,26 @@ function expandArtifacts(artifacts, includeSpeedline) {

// currently only trace logs and performance logs should be imported
if (artifacts.traces) {
let trace;
Object.keys(artifacts.traces).forEach(key => {
if (artifacts.traces[key].traceContents) {
log.log('info', 'Normalizng trace contents into expected state...');
trace = require(artifacts.traces[key].traceContents);

expandedArtifacts.traces[key].traceContents = cleanTrace(trace.traceEvents || trace);
log.log('info', 'Normalizng trace contents into expected state...');
let trace = require(artifacts.traces[key]);
// Before Chrome 54.0.2816 (codereview.chromium.org/2161583004), trace was
// an array of trace events. After this point, trace is an object with a
// traceEvents property. Normalize to new format.
if (Array.isArray(trace)) {
trace = {
traceEvents: trace
};
}
cleanTrace(trace.traceEvents);

expandedArtifacts.traces[key] = trace;
});
}

if (includeSpeedline) {
const speedline = new SpeedlineGatherer();
speedline.afterPass({}, {traceContents: expandedArtifacts.traces.defaultPass.traceContents});
speedline.afterPass({}, {traceEvents: expandedArtifacts.traces.defaultPass.traceEvents});
expandedArtifacts.Speedline = speedline.artifact;
}

Expand Down
23 changes: 15 additions & 8 deletions lighthouse-core/gather/gather-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,18 @@ class GatherRunner {
pass = pass.then(_ => {
log.log('status', `Gathering: trace "${traceName}"`);
return driver.endTrace().then(traceContents => {
loadData.traces[traceName] = {traceContents};
// Before Chrome 54.0.2816 (codereview.chromium.org/2161583004),
// traceContents was an array of trace events. After this point,
// traceContents is an object with a traceEvents property. Normalize
// to new format.
if (Array.isArray(traceContents)) {
traceContents = {
traceEvents: traceContents
};
}

loadData.traces[traceName] = traceContents;
loadData.traceEvents = traceContents.traceEvents;
log.verbose('statusEnd', `Gathering: trace "${traceName}"`);
});
});
Expand All @@ -140,9 +151,6 @@ class GatherRunner {
return chain.then(_ => {
const status = `Gathering: ${gatherer.name}`;
log.log('status', status);
if (config.trace) {
loadData.traceContents = loadData.traces[traceName].traceContents;
}
return Promise.resolve(gatherer.afterPass(options, loadData)).then(ret => {
log.verbose('statusEnd', status);
return ret;
Expand Down Expand Up @@ -197,10 +205,9 @@ class GatherRunner {
.then(_ => this.pass(runOptions))
.then(_ => this.afterPass(runOptions))
.then(loadData => {
// Need to manually merge traces property before
// merging loadDat into tracingData to avoid data loss.
Object.assign(loadData.traces, tracingData.traces);
Object.assign(tracingData, loadData);
// Merge pass trace and network data into tracingData.
Object.assign(tracingData.traces, loadData.traces);
tracingData.networkRecords = loadData.networkRecords;
})
.then(_ => this.tearDown(runOptions));
}, Promise.resolve());
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/gather/gatherers/screenshots.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class ScreenshotFilmstrip extends Gatherer {
}

afterPass(options, tracingData) {
return this.getScreenshots(tracingData.traceContents).then(screenshots => {
return this.getScreenshots(tracingData.traceEvents).then(screenshots => {
this.artifact = screenshots;
});
}
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/gather/gatherers/speedline.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const speedline = require('speedline');
class Speedline extends Gatherer {

afterPass(options, tracingData) {
return speedline(tracingData.traceContents).then(results => {
return speedline(tracingData.traceEvents).then(results => {
this.artifact = results;
}).catch(err => {
this.artifact = {
Expand Down
6 changes: 5 additions & 1 deletion lighthouse-core/lib/asset-saver.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,11 @@ function saveArtifacts(artifacts, filename) {
}

function prepareAssets(options, artifacts) {
const traceData = filterForSize(artifacts.traceContents);
const traceData = Object.keys(artifacts.traces).map(traceName => {
const filteredTrace = Object.assign({}, artifacts.traces[traceName]);
filteredTrace.traceEvents = filterForSize(filteredTrace.traceEvents);
return filteredTrace;
});
const html = screenshotDump(options, artifacts.ScreenshotFilmstrip);
return {traceData, html};
}
Expand Down
6 changes: 3 additions & 3 deletions lighthouse-core/test/audits/estimated-input-latency.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@

const Audit = require('../../audits/estimated-input-latency.js');
const assert = require('assert');
const traceContents = require('../fixtures/traces/progressive-app.json');
const traceEvents = require('../fixtures/traces/progressive-app.json');

/* eslint-env mocha */

describe('Performance: estimated-input-latency audit', () => {
it('scores a -1 with invalid trace data', () => {
const output = Audit.audit({
traces: {[Audit.DEFAULT_TRACE]: {traceContents: '[{"pid": 15256,"tid": 1295,"t'}},
traces: {[Audit.DEFAULT_TRACE]: {traceEvents: '[{"pid": 15256,"tid": 1295,"t'}},
Speedline: {
first: 500
}
Expand All @@ -35,7 +35,7 @@ describe('Performance: estimated-input-latency audit', () => {

it('evaluates valid input correctly', () => {
const output = Audit.audit({
traces: {[Audit.DEFAULT_TRACE]: {traceContents}},
traces: {[Audit.DEFAULT_TRACE]: {traceEvents}},
Speedline: {
first: 500
}
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/test/audits/first-meaningful-paint.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

const Audit = require('../../audits/first-meaningful-paint.js');
const assert = require('assert');
const traceEvents = require('../fixtures/traces/progressive-app.json');

/* eslint-env mocha */
describe('Performance: first-meaningful-paint audit', () => {
Expand All @@ -36,9 +37,8 @@ describe('Performance: first-meaningful-paint audit', () => {
let fmpResult;

it('processes a valid trace file', done => {
const traceData = require('../fixtures/traces/progressive-app.json');
assert.doesNotThrow(_ => {
Audit.audit({traces: {[Audit.DEFAULT_TRACE]: {traceContents: traceData}}})
Audit.audit({traces: {[Audit.DEFAULT_TRACE]: {traceEvents}}})
.then(response => {
fmpResult = response;
done();
Expand Down
8 changes: 4 additions & 4 deletions lighthouse-core/test/audits/time-to-interactive.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const Audit = require('../../audits/time-to-interactive.js');
const SpeedlineGather = require('../../gather/gatherers/speedline');
const assert = require('assert');

const traceContents = require('../fixtures/traces/progressive-app.json');
const traceEvents = require('../fixtures/traces/progressive-app.json');
const speedlineGather = new SpeedlineGather();

/* eslint-env mocha */
Expand All @@ -28,7 +28,7 @@ describe('Performance: time-to-interactive audit', () => {
return Audit.audit({
traces: {
[Audit.DEFAULT_TRACE]: {
traceContents: '[{"pid": 15256,"tid": 1295,"t'
traceEvents: '[{"pid": 15256,"tid": 1295,"t'
}
},
Speedline: {
Expand All @@ -41,12 +41,12 @@ describe('Performance: time-to-interactive audit', () => {
});

it('evaluates valid input correctly', () => {
let artifacts = {traceContents};
let artifacts = {traceEvents};
return speedlineGather.afterPass({}, artifacts).then(_ => {
artifacts.Speedline = speedlineGather.artifact;
// This is usually done by the driver
artifacts.traces = {
[Audit.DEFAULT_TRACE]: {traceContents}
[Audit.DEFAULT_TRACE]: {traceEvents}
};
return Audit.audit(artifacts).then(output => {
assert.equal(output.rawValue, '1105.8');
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/test/audits/user-timing.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

const Audit = require('../../audits/user-timings.js');
const assert = require('assert');
const traceContents = require('../fixtures/traces/trace-user-timings.json');
const traceEvents = require('../fixtures/traces/trace-user-timings.json');

/* eslint-env mocha */

Expand All @@ -29,7 +29,7 @@ describe('Performance: user-timings audit', () => {
});

it('evaluates valid input correctly', () => {
return Audit.audit({traces: {[Audit.DEFAULT_TRACE]: {traceContents}}})
return Audit.audit({traces: {[Audit.DEFAULT_TRACE]: {traceEvents}}})
.then(response => {
assert.equal(response.score, 2);
assert.ok(!Number.isNaN(response.extendedInfo.value[0].startTime));
Expand Down
26 changes: 9 additions & 17 deletions lighthouse-core/test/config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,13 @@ describe('Config', () => {
const config = new Config({
artifacts: {
traces: {
defaultPass: {
traceContents: path.resolve(__dirname, '../fixtures/traces/trace-user-timings.json')
}
defaultPass: path.resolve(__dirname, '../fixtures/traces/trace-user-timings.json')
},
performanceLog: path.resolve(__dirname, '../fixtures/perflog.json')
}
});
const traceUserTimings = require('../fixtures/traces/trace-user-timings.json');
assert.deepStrictEqual(config.artifacts.traces.defaultPass.traceContents, traceUserTimings);
assert.deepStrictEqual(config.artifacts.traces.defaultPass.traceEvents, traceUserTimings);
assert.ok(config.artifacts.CriticalRequestChains);
assert.ok(config.artifacts.CriticalRequestChains['93149.1']);
assert.ok(config.artifacts.CriticalRequestChains['93149.1'].request);
Expand All @@ -150,27 +148,23 @@ describe('Config', () => {
const config = new Config({
artifacts: {
traces: {
defaultPass: {
traceContents: path.resolve(__dirname,
'../fixtures/traces/trace-user-timings-no-tracingstartedinpage.json')
}
defaultPass: path.resolve(__dirname,
'../fixtures/traces/trace-user-timings-no-tracingstartedinpage.json')
},
performanceLog: path.resolve(__dirname, '../fixtures/perflog.json')
}
});

assert.ok(config.artifacts.traces.defaultPass.traceContents.find(
assert.ok(config.artifacts.traces.defaultPass.traceEvents.find(
e => e.name === 'TracingStartedInPage' && e.args.data.page === '0xhad00p'));
});

it('doesnt add speedline artifact to tests without tti audit', () => {
const config = new Config({
artifacts: {
traces: {
defaultPass: {
traceContents: path.resolve(__dirname,
'../fixtures/traces/trace-user-timings-no-tracingstartedinpage.json')
}
defaultPass: path.resolve(__dirname,
'../fixtures/traces/trace-user-timings-no-tracingstartedinpage.json')
},
performanceLog: path.resolve(__dirname, '../fixtures/perflog.json')
},
Expand All @@ -187,10 +181,8 @@ describe('Config', () => {
const config = new Config({
artifacts: {
traces: {
defaultPass: {
traceContents: path.resolve(__dirname,
'../fixtures/traces/trace-user-timings-no-tracingstartedinpage.json')
}
defaultPass: path.resolve(__dirname,
'../fixtures/traces/trace-user-timings-no-tracingstartedinpage.json')
},
performanceLog: path.resolve(__dirname, '../fixtures/perflog.json')
},
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/test/gather/gather-runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ describe('GatherRunner', function() {

return GatherRunner.afterPass({driver, config}).then(vals => {
assert.equal(calledTrace, true);
assert.deepEqual(vals.traces[Audit.DEFAULT_TRACE].traceContents, {x: 1});
assert.deepEqual(vals.traces[Audit.DEFAULT_TRACE], {x: 1});
});
});

Expand All @@ -180,7 +180,7 @@ describe('GatherRunner', function() {
};

return GatherRunner.afterPass({driver, config}).then(vals => {
assert.deepEqual(vals.traces.notTheDefaultPass.traceContents, {x: 1});
assert.deepEqual(vals.traces.notTheDefaultPass, {x: 1});
});
});

Expand Down
5 changes: 3 additions & 2 deletions lighthouse-core/test/gather/gatherers/screenshots.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,16 @@

const ScreenshotsGather = require('../../../gather/gatherers/screenshots');
const assert = require('assert');
const traceEvents = require('../../fixtures/traces/progressive-app.json');

let screenshotsGather = new ScreenshotsGather();

describe('Screenshot gatherer', () => {
it('returns an artifact for a real trace', () => {
const traceData = require('../../fixtures/traces/progressive-app.json');
// Currently this test must rely on knowing the phase hook for the gatherer.
// A little unfortunate, but we need a "run scheduler with this gatherer, this mocked driver,
// and this trace" test class to do that right
return screenshotsGather.afterPass(undefined, {traceContents: traceData}).then(_ => {
return screenshotsGather.afterPass(undefined, {traceEvents}).then(_ => {
assert.ok(Array.isArray(screenshotsGather.artifact));
assert.equal(screenshotsGather.artifact.length, 7);

Expand Down
Loading

0 comments on commit 7c9c44f

Please sign in to comment.