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.

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

Add test cases for perf-x database #1597

Merged
merged 3 commits into from
Feb 2, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,11 @@ const rimraf = require('rimraf');
const assetSaver = require('../../../lighthouse-core/lib/asset-saver');

class ExperimentDatabase {
constructor(url, config) {
this._url = url;
this._config = config;

constructor() {
this._fsRoot = fs.mkdtempSync(`${__dirname}/experiment-data-`);
this._timeStamps = {};
}

get url() {
return this._url;
}

get config() {
return this._config;
}

get timeStamps() {
return this._timeStamps;
}
Expand Down
13 changes: 9 additions & 4 deletions lighthouse-cli/performance-experiment/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ const PerfXReportGenerator = require('./report/perf-x-report-generator');

let database;
let fallbackReportId;
let url;
let config;
/**
* Start the server with an arbitrary port and open report page in the default browser.
* @param {!Object} params A JSON contains lighthouse parameters
Expand All @@ -46,7 +48,10 @@ let fallbackReportId;
*/
function hostExperiment(params, results) {
return new Promise(resolve => {
database = new ExperimentDatabase(params.url, params.config);
url = params.url;
config = params.config;

database = new ExperimentDatabase();
const id = database.saveData(params.flags, results);
fallbackReportId = id;

Expand Down Expand Up @@ -101,10 +106,10 @@ function reportRequestHandler(request, response) {

const reportsMetadata = Object.keys(database.timeStamps).map(key => {
const generatedTime = database.timeStamps[key];
return {url: database.url, reportHref: `/?id=${key}`, generatedTime};
return {url, reportHref: `/?id=${key}`, generatedTime};
});
reportsMetadata.sort((metadata1, metadata2) => {
return metadata1.generatedTime - metadata2.generatedTime;
return new Date(metadata1.generatedTime) - new Date(metadata2.generatedTime);
});
const reportsCatalog = {reportsMetadata, selectedReportHref: `/?id=${id}`};

Expand Down Expand Up @@ -137,7 +142,7 @@ function rerunRequestHandler(request, response) {
const additionalFlags = JSON.parse(message);
Object.assign(flags, additionalFlags);

lighthouse(database.url, flags, database.config).then(results => {
lighthouse(url, flags, config).then(results => {
results.artifacts = undefined;
const id = database.saveData(flags, results);
response.writeHead(200);
Expand Down
131 changes: 131 additions & 0 deletions lighthouse-cli/test/cli/performance-experiment/database-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/**
* Copyright 2017 Google Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

'use strict';

/* eslint-env mocha */
const assert = require('assert');

const PerfXDatabase = require('../../../performance-experiment/experiment-database/database');
const sampleResults = require('../../../../lighthouse-core/test/results/sample');

describe('Perf-X Database', function() {
it('can store and output experiments data', () => {
const perfXDatabase = new PerfXDatabase();
process.on('exit', () => perfXDatabase.clear());
Copy link
Member

Choose a reason for hiding this comment

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

i'd put this in an afterEach block (some other tests have them)

Copy link
Member

Choose a reason for hiding this comment

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

and do you want to use a fresh database for each of these test cases? if so you could do setup/teardown in beforeEach/afterEach inside the describe and do each test case in their own it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'd put this in an afterEach block (some other tests have them)

I didn't know that! Thx for your advice

and do you want to use a fresh database for each of these test cases?

Are you referring to testCases or 'it'? I want to test the whether database is capable of storing multiple data sets here. I will rename testCases to dataSets to make it clear.


const testCases = [
{
flags: {
'blockedUrlPatterns': ['.woff2', '.jpg'],
'disable-network-throttling': false
},
results: sampleResults
},
{
flags: {
'blockedUrlPatterns': ['.woff'],
'disable-cpu-throttling': true,
'deep-reference': {'an-object': {'an array': ['something', 'an element']}}
},
results: {
generatedTime: new Date(2015, 6, 37, 0, 12, 55, 60).toJSON(),
url: 'http://google.com/',
else: 'some-data'
}
},
{
flags: {
'blockedUrlPatterns': ['.woff', 'cat.jpg', '*'],
Copy link
Member

Choose a reason for hiding this comment

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

pretty odd we have this mix of camel casing and not in the flags. Why arent the disable flags in camelcase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

'disable-cpu-throttling': false,
'disable-device-emulation': true
},
results: {
'generatedTime': new Date(2014, 5, 36, 23, 56, 54, 99).toJSON(),
'url': 'http://mdn.com/',
'audits': [{'is-on-https': {'score': true}}],
}
}
];

testCases.forEach(testCase => {
testCase.key = perfXDatabase.saveData(testCase.flags, testCase.results);
});

testCases.forEach(testCase => {
assert.deepStrictEqual(perfXDatabase.getFlags(testCase.key), testCase.flags);
assert.deepStrictEqual(perfXDatabase.getResults(testCase.key), testCase.results);
});
});

it('prevents data from being changed by reference', () => {
const perfXDatabase = new PerfXDatabase();
process.on('exit', () => perfXDatabase.clear());

const flags = {
'blockedUrlPatterns': ['.woff', '.jpg', 'random'],
'disable-cpu-throttling': false,
'some-random-flag': 'some random value'
};
const results = JSON.parse(JSON.stringify(sampleResults));

const key = perfXDatabase.saveData(flags, results);
const flagsBeforeChange = JSON.parse(JSON.stringify(perfXDatabase.getFlags(key)));
const resultsBeforeChange = JSON.parse(JSON.stringify(perfXDatabase.getResults(key)));

// data won't be changed when the falgs/results passed to perfXDatabase.saveData is changed
Copy link
Member

Choose a reason for hiding this comment

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

falgs => flags

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

flags.blockedUrlPatterns.push('something');
results.url = undefined;

// data won't be changed when the falgs/results returned by perfXDatabase is changed
perfXDatabase.getFlags(key)['another attribute'] = 'random value';
perfXDatabase.getResults(key).aggregations.push('something-else');

assert.deepStrictEqual(perfXDatabase.getFlags(key), flagsBeforeChange);
assert.deepStrictEqual(perfXDatabase.getResults(key), resultsBeforeChange);
});

it('returns correct timestamps', () => {
const perfXDatabase = new PerfXDatabase();
process.on('exit', () => perfXDatabase.clear());

const testCases = [
{
results: sampleResults
},
{
results: {
generatedTime: new Date(2015, 6, 37, 0, 12, 55, 60).toJSON(),
url: 'http://google.com/',
}
},
{
results: {
'generatedTime': new Date(2014, 5, 36, 23, 56, 54, 99).toJSON(),
'url': 'http://mdn.com/',
}
}
];

testCases.forEach(testCase => {
testCase.key = perfXDatabase.saveData({}, testCase.results);
});

testCases.forEach(testCase => {
assert.strictEqual(perfXDatabase.timeStamps[testCase.key], testCase.results.generatedTime);
});
Copy link
Member

Choose a reason for hiding this comment

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

how about an assertion about perfXDatabase.timeStamps.length? (3 is expected)

(I presume this would fail right now, until the process.on exit is changed to an afterEach..)

Copy link
Contributor Author

@weiwei-lin weiwei-lin Feb 1, 2017

Choose a reason for hiding this comment

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

how about an assertion about perfXDatabase.timeStamps.length? (3 is expected)

Thx for your advice. I will add that.

});
});