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

clients(lr): allow for custom config #7613

Merged
merged 6 commits into from
Apr 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
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
19 changes: 13 additions & 6 deletions clients/lightrider-entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@ const LR_PRESETS = {

/**
* Run lighthouse for connection and provide similar results as in CLI.
*
* If configOverride is provided, lrDevice and categoryIDs are ignored.
* @param {Connection} connection
* @param {string} url
* @param {LH.Flags} flags Lighthouse flags, including `output`
* @param {{lrDevice?: 'desktop'|'mobile', categoryIDs?: Array<string>, logAssets: boolean, keepRawValues: boolean}} lrOpts Options coming from Lightrider
* @param {{lrDevice?: 'desktop'|'mobile', categoryIDs?: Array<string>, logAssets: boolean, keepRawValues: boolean, configOverride?: LH.Config.Json}} lrOpts Options coming from Lightrider
patrickhulce marked this conversation as resolved.
Show resolved Hide resolved
* @return {Promise<string|Array<string>|void>}
*/
async function runLighthouseInLR(connection, url, flags, lrOpts) {
const {lrDevice, categoryIDs, logAssets, keepRawValues} = lrOpts;
const {lrDevice, categoryIDs, logAssets, keepRawValues, configOverride} = lrOpts;

// Certain fixes need to kick in under LR, see https://github.com/GoogleChrome/lighthouse/issues/5839
global.isLightrider = true;
Expand All @@ -38,10 +40,15 @@ async function runLighthouseInLR(connection, url, flags, lrOpts) {
flags.logLevel = flags.logLevel || 'info';
flags.channel = 'lr';

const config = lrDevice === 'desktop' ? LR_PRESETS.desktop : LR_PRESETS.mobile;
if (categoryIDs) {
config.settings = config.settings || {};
config.settings.onlyCategories = categoryIDs;
let config;
if (configOverride) {
config = configOverride;
} else {
config = lrDevice === 'desktop' ? LR_PRESETS.desktop : LR_PRESETS.mobile;
if (categoryIDs) {
config.settings = config.settings || {};
config.settings.onlyCategories = categoryIDs;
}
}

try {
Expand Down
48 changes: 48 additions & 0 deletions clients/test/lightrider-entry-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,53 @@ describe('lightrider-entry', () => {

runStub.mockRestore();
});

it('uses the desktop config preset when device is desktop', async () => {
const runStub = jest.spyOn(Runner, 'run');

const mockConnection = {};
const url = 'https://example.com';

const lrDevice = 'desktop';
await lhBackground.runLighthouseInLR(mockConnection, url, {}, {lrDevice});
const config = runStub.mock.calls[0][1].config;
assert.equal(config.settings.emulatedFormFactor, 'desktop');

runStub.mockRestore();
});

it('uses the mobile config preset when device is mobile', async () => {
const runStub = jest.spyOn(Runner, 'run');

const mockConnection = {};
const url = 'https://example.com';

const lrDevice = 'mobile';
await lhBackground.runLighthouseInLR(mockConnection, url, {}, {lrDevice});
const config = runStub.mock.calls[0][1].config;
assert.equal(config.settings.emulatedFormFactor, 'mobile');

runStub.mockRestore();
});

it('overrides the default config when one is provided', async () => {
const runStub = jest.spyOn(Runner, 'run');

const mockConnection = {};
const url = 'https://example.com';

const configOverride = {
default: 'lighthouse:default',
settings: {
onlyAudits: ['network-requests'],
},
};
await lhBackground.runLighthouseInLR(mockConnection, url, {}, {configOverride});
const config = runStub.mock.calls[0][1].config;
assert.equal(config.settings.onlyAudits.length, 1);
assert.equal(config.settings.onlyAudits[0], 'network-requests');

runStub.mockRestore();
});
});
});
17 changes: 12 additions & 5 deletions lighthouse-core/test/report/proto-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,21 +55,28 @@ describe('round trip JSON comparison subsets', () => {
});

it('has the same config values', () => {
expect(roundTripJson.configSettings).toMatchObject(sampleJson.configSettings);
// Config settings from proto round trip should be a subset of the actual settings.
expect(sampleJson.configSettings).toMatchObject(roundTripJson.configSettings);
Copy link
Collaborator

Choose a reason for hiding this comment

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

wait how did it work the other way around then if one is a subset of the other 😆

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These objects are actually 100% equal, so either way would be fine. It was only apparent that the order is wrong when I was allowing more properties to be in the proto result.

so the test change isn't needed, but it is more correct. whatever settings are in the round trip json (from proto) should match the values as found in the originating settings json

});
});

// Note: In a failing diff, if you see details.summary going from {} to null, it's OK.
// Jest considers this not a failure, and neither do we, here in the python roundtrip
// Meanwhile, The PSI roundtrip maintains {} to {}.
describe('round trip JSON comparison to everything', () => {
let sampleJson;

beforeEach(() => {
sampleJson = JSON.parse(preprocessor.processForProto(sample));

// Proto conversion turns empty summaries into null. This is OK,
// and is handled in the PSI roundtrip just fine, but messes up the easy
// jest sub-object matcher. So, we put the empty object back in its place.
for (const audit of Object.values(roundTripJson.audits)) {
if (audit.details && audit.details.summary === null) {
audit.details.summary = {};
}
}
});

it('has the same JSON overall', () => {
expect(roundTripJson).toMatchObject(sampleJson);
expect(sampleJson).toMatchObject(roundTripJson);
});
});