-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Switch to new puppeteer APIs for emulating conditions #33410
Switch to new puppeteer APIs for emulating conditions #33410
Conversation
@@ -187,28 +187,34 @@ async function simulateAdverseConditions() { | |||
return; | |||
} | |||
|
|||
const client = await page.target().createCDPSession(); | |||
if ( OFFLINE ) { | |||
await page.setOfflineMode( true ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs aren't very clear, but I this this is needed if we ever want to test a PWA. But I don't fully know what it does.
Size Change: 0 B Total Size: 1.07 MB ℹ️ View Unchanged
|
await page.emulateNetworkConditions( { | ||
// Download speed (bytes/s) | ||
downloadThroughput: ( ( 1.6 * 1024 * 1024 ) / 8 ) * 0.9, | ||
download: ( ( 1.6 * 1024 * 1024 ) / 8 ) * 0.9, | ||
// Upload speed (bytes/s) | ||
uploadThroughput: ( ( 750 * 1024 ) / 8 ) * 0.9, | ||
upload: ( ( 750 * 1024 ) / 8 ) * 0.9, | ||
// Latency (ms) | ||
latency: 150 * 3.75, | ||
} ); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can just use the predefined conditions here?
await page.emulateNetworkConditions( puppeteer.networkConditions['Fast 3G'] );
Then, we can remove the comments above too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might seem silly, but the puppeteer object can't be imported.
Neither of these seem to work:
import puppeteer from 'puppeteer';
import puppeteer from 'puppeteer-core';
Not sure if you have a quick fix, but I don't want to mess around too much with dependencies, much more important stuff to do, so I think let's just ship this as is for now. Or feel free to push a commit if you can solve it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, seems like a npm/eslint issue. Peer dependency should be installed by default. Upgrading to npm 7 might fix that. But sure, this is good enough 👍.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Description
Some of the puppeteer functionality we use in tests has been replaced with friendlier APIs, that don't require calling
page.target().createCDPSession();
.This PR updates them.
How has this been tested?
it.only
to make only that test runOFFLINE=true npm run test-e2e:watch -- --puppeteer-interactive
SLOW_NETWORK=true npm run test-e2e:watch -- --puppeteer-interactive
THROTTLE_CPU=200 npm run test-e2e:watch -- --puppeteer-interactive
Types of changes
Small refactor