Skip to content

Commit

Permalink
Fixes TTI not being counted in overall score
Browse files Browse the repository at this point in the history
Better checking of aggregation config to prevent happening again.
Changes TTI audit rawValue to be a float not string.
  • Loading branch information
paullewis authored and brendankenny committed Aug 17, 2016
1 parent 916b335 commit 82c5051
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 16 deletions.
17 changes: 11 additions & 6 deletions lighthouse-core/aggregator/aggregate.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,24 +67,28 @@ class Aggregate {
* @private
* @param {!AuditResult} result The audit's output value.
* @param {!AggregationCriterion} expected The aggregation's expected value and weighting for this result.
* @param {!string} name The name of the audit.
* @return {number} The weighted result.
*/
static _convertToWeight(result, expected) {
static _convertToWeight(result, expected, name) {
let weight = 0;

if (typeof expected === 'undefined' ||
typeof expected.rawValue === 'undefined' ||
typeof expected.weight === 'undefined') {
return weight;
const msg =
`Config for ${name} is undefined or does not contain rawValue and weight properties`;
throw new Error(msg);
}

if (typeof result === 'undefined' ||
typeof result.rawValue === 'undefined' ||
typeof result.score === 'undefined') {
return weight;
const msg =
`Audit result for ${name} is undefined or does not contain score property`;
throw new Error(msg);
}

if (typeof result.rawValue !== typeof expected.rawValue) {
if (typeof result.score !== typeof expected.rawValue) {
const msg =
`Expected rawValue of type ${typeof expected.rawValue}, got ${typeof result.rawValue}`;
throw new Error(msg);
Expand Down Expand Up @@ -181,7 +185,8 @@ class Aggregate {

overallScore += Aggregate._convertToWeight(
filteredAndRemappedResults[e],
item.criteria[e]);
item.criteria[e],
e);
});

return {
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/audits/time-to-interactive.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,15 @@ class TTIMetric extends Audit {
// Grab this latency and try the threshold again
currentLatency = estLatency;
}
const timeToInteractive = startTime.toFixed(1);
const timeToInteractive = parseFloat(startTime.toFixed(1));

// Use the CDF of a log-normal distribution for scoring.
// < 1200ms: score≈100
// 5000ms: score=50
// >= 15000ms: score≈0
const distribution = TracingProcessor.getLogNormalDistribution(SCORING_MEDIAN,
SCORING_POINT_OF_DIMINISHING_RETURNS);
let score = 100 * distribution.computeComplementaryPercentile(timeToInteractive);
let score = 100 * distribution.computeComplementaryPercentile(startTime);

// Clamp the score to 0 <= x <= 100.
score = Math.min(100, score);
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/config/default.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@
"weight": 1
},
"time-to-interactive": {
"value": 100,
"rawValue": 100,
"weight": 1
},
"scrolling-60fps": {
Expand Down
53 changes: 46 additions & 7 deletions lighthouse-core/test/aggregator/aggregate.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,16 +125,16 @@ describe('Aggregate', () => {
'Cannot remap: test already exists');
});

it('returns a weight of zero for undefined inputs', () => {
return assert.equal(Aggregate._convertToWeight(), 0);
it('throws for undefined inputs', () => {
return assert.throws(_ => Aggregate._convertToWeight(), 0);
});

it('returns a weight of zero for undefined results', () => {
it('throws for undefined results', () => {
const expected = {
rawValue: true,
weight: 10
};
return assert.equal(Aggregate._convertToWeight(undefined, expected), 0);
return assert.throws(_ => Aggregate._convertToWeight(undefined, expected));
});

it('returns a weight of zero for undefined expectations', () => {
Expand All @@ -143,7 +143,7 @@ describe('Aggregate', () => {
score: true,
displayValue: ''
};
return assert.equal(Aggregate._convertToWeight(result, undefined), 0);
return assert.throws(_ => Aggregate._convertToWeight(result, undefined));
});

it('returns the correct weight for a boolean result', () => {
Expand Down Expand Up @@ -176,7 +176,7 @@ describe('Aggregate', () => {
return assert.equal(Aggregate._convertToWeight(result, expected), 5);
});

it('returns the a weight of zero if weight is missing from the expected', () => {
it('throws if weight is missing from the expected', () => {
const expected = {
rawValue: 100
};
Expand All @@ -187,7 +187,7 @@ describe('Aggregate', () => {
displayValue: '50'
};

return assert.equal(Aggregate._convertToWeight(result, expected), 0);
return assert.throws(_ => Aggregate._convertToWeight(result, expected), 0);
});

it('returns a weight of zero for other inputs', () => {
Expand Down Expand Up @@ -296,6 +296,45 @@ describe('Aggregate', () => {
});
});

it('throws when given a result containing no score property', () => {
const items = [{
criteria: {
test: {
rawValue: true,
weight: 1
}
}
}];

const results = [{
name: 'test',
value: 'should be rawValue',
displayValue: ''
}];
const scored = true;

return assert.throws(_ => Aggregate.compare(results, items, scored));
});

it('throws when given a criterion containing no rawValue property', () => {
const items = [{
criteria: {
test: {
weight: 1
}
}
}];

const results = [{
name: 'test',
score: false,
displayValue: ''
}];
const scored = true;

return assert.throws(_ => Aggregate.compare(results, items, scored));
});

it('filters a set correctly', () => {
const items = [{
criteria: {
Expand Down

0 comments on commit 82c5051

Please sign in to comment.