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.

Sign up for GitHub

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

core(emulation): use Moto G4 for mobile emulation #10191

Merged
merged 6 commits into from
Jan 22, 2020
Merged

core(emulation): use Moto G4 for mobile emulation #10191

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0d1f549
core(emulation): use Moto G4 for mobile emulation
connorjclark Jan 8, 2020
39a4101
3
connorjclark Jan 15, 2020
748d005
Merge remote-tracking branch 'origin/master' into motog4
connorjclark Jan 15, 2020
4a8c369
fix
connorjclark Jan 15, 2020
d7e2b22
update
connorjclark Jan 15, 2020
d294ea8
fixsmoke
connorjclark Jan 15, 2020
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
21 changes: 10 additions & 11 deletions lighthouse-core/lib/emulation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,18 @@
/** @typedef {import('../gather/driver.js')} Driver */

/**
* Nexus 5X metrics adapted from emulated_devices/module.json
* @type {LH.Crdp.Emulation.SetDeviceMetricsOverrideRequest}
*/
const NEXUS5X_EMULATION_METRICS = {
const MOTOG4_EMULATION_METRICS = {
mobile: true,
screenWidth: 412,
screenHeight: 660,
width: 412,
height: 660,
screenWidth: 360,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Need to look at an actual device and see what a browser reports this as.

Copy link
Collaborator

Choose a reason for hiding this comment

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

360 looks right https://github.com/WPO-Foundation/webpagetest/blob/901d2906711c32577f9c8e19603cf491096fcbf8/www/settings/mobile_devices.ini#L7

could use a WPT run if you need to to verify

screenHeight: 640,
width: 360,
height: 640,
positionX: 0,
positionY: 0,
scale: 1,
deviceScaleFactor: 2.625,
deviceScaleFactor: 2.625, // ??
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

what the heck is this @paulirish

Copy link
Collaborator

@patrickhulce patrickhulce Jan 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

that should be the device DPR

Copy link
Collaborator

Choose a reason for hiding this comment

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

looks like it's 3 for moto g4

https://github.com/WPO-Foundation/webpagetest/blob/901d2906711c32577f9c8e19603cf491096fcbf8/www/settings/mobile_devices.ini#L9

Copy link
Collaborator

Choose a reason for hiding this comment

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

math checks out too 640 * 3 = 1920, 360 * 3 = 1080

screenOrientation: {
angle: 0,
type: 'portraitPrimary',
Expand All @@ -39,7 +38,7 @@ const DESKTOP_EMULATION_METRICS = {
};

// eslint-disable-next-line max-len
const NEXUS5X_USERAGENT = 'Mozilla/5.0 (Linux; Android 6.0.1; Nexus 5 Build/MRA58N) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.3963.0 Mobile Safari/537.36 Chrome-Lighthouse';
const MOTOG4_USERAGENT = 'Mozilla/5.0 (Linux; Android 6.0.1; Moto G (4)) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.3963.0 Mobile Safari/537.36 Chrome-Lighthouse';
patrickhulce marked this conversation as resolved.
Show resolved Hide resolved
// eslint-disable-next-line max-len
const DESKTOP_USERAGENT = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.3963.0 Safari/537.36 Chrome-Lighthouse';

Expand All @@ -64,8 +63,8 @@ const NO_CPU_THROTTLE_METRICS = {

const emulationParams = {
mobile: {
userAgent: NEXUS5X_USERAGENT,
metrics: NEXUS5X_EMULATION_METRICS,
userAgent: MOTOG4_USERAGENT,
metrics: MOTOG4_EMULATION_METRICS,
touchEnabled: true,
},
desktop: {
Expand Down Expand Up @@ -161,6 +160,6 @@ module.exports = {
enableCPUThrottling,
disableCPUThrottling,
goOffline,
MOBILE_USERAGENT: NEXUS5X_USERAGENT,
MOBILE_USERAGENT: MOTOG4_USERAGENT,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nice thinking here on the export

DESKTOP_USERAGENT,
};
2 changes: 1 addition & 1 deletion lighthouse-core/report/html/renderer/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ class Util {
}

let deviceEmulation = 'No emulation';
if (settings.emulatedFormFactor === 'mobile') deviceEmulation = 'Emulated Nexus 5X';
if (settings.emulatedFormFactor === 'mobile') deviceEmulation = 'Emulated Moto G4';
if (settings.emulatedFormFactor === 'desktop') deviceEmulation = 'Emulated Desktop';

return {
Expand Down
8 changes: 4 additions & 4 deletions lighthouse-core/test/report/html/renderer/report-renderer-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,10 @@ describe('ReportRenderer', () => {
assert.ok(descriptions.length >= 3);

const descriptionsTxt = descriptions.map(el => el.textContent).join('\n');
assert.ok(/Nexus/.test(descriptionsTxt), 'should have added device emulation');
assert.ok(/RTT/.test(descriptionsTxt), 'should have added network');
assert.ok(/\dx/.test(descriptionsTxt), 'should have added CPU');
assert.ok(descriptionsTxt.includes(sampleResults.userAgent), 'user agent populated');
expect(descriptionsTxt).toContain('Moto G4');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

asserts are bad.

expect(descriptionsTxt).toContain('RTT');
expect(descriptionsTxt).toMatch(/\dx/);
expect(descriptionsTxt).toContain(sampleResults.userAgent);
});
});

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/report/html/renderer/util-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe('util helpers', () => {
it('builds device emulation string', () => {
const get = opts => Util.getEmulationDescriptions(opts).deviceEmulation;
assert.equal(get({emulatedFormFactor: 'none'}), 'No emulation');
assert.equal(get({emulatedFormFactor: 'mobile'}), 'Emulated Nexus 5X');
assert.equal(get({emulatedFormFactor: 'mobile'}), 'Emulated Moto G4');
assert.equal(get({emulatedFormFactor: 'desktop'}), 'Emulated Desktop');
});

Expand Down