-
Notifications
You must be signed in to change notification settings - Fork 14
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
Fix test runner, smarter server #168
Conversation
|
||
server.listen(process.env.PORT || 8080, () => { | ||
const { address, port } = server.address(); | ||
Server.origin = `http://[${address}]:${port}`; |
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.
We use introspection to generate the origin
component necessary to qualify the path and leverage URL
above for smarter path parsing.
static requestListener(request, response) { | ||
if (request.method === 'GET') { | ||
const fileUrl = request.url; | ||
const filePath = path.resolve(`${root}/${fileUrl}`); | ||
const url = new URL(request.url, Server.origin); |
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.
By adopting URL
here we can easily extract pathname
and fix the confusion around paths which incorporate query params. Easy peasy.
@@ -3,7 +3,17 @@ import puppeteer from 'puppeteer'; | |||
(async () => { | |||
try { | |||
// Open our browser. | |||
const browser = await puppeteer.launch({ timeout: 10000 }); | |||
const browser = await puppeteer.launch({ |
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.
These are just a few flags I borrowed from another project (Profiler)
timeout: 10000, | ||
// opt-in to the new Chrome headless implementation | ||
// ref: https://developer.chrome.com/articles/new-headless/ | ||
headless: 'new', |
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.
Oh, we using the new, new. Cool.
args: [ | ||
// Disables interactive prompt: Do you want to the application Chromium.app to accept incoming network connections? | ||
// ref: https://github.com/puppeteer/puppeteer/issues/4752#issuecomment-586599843 | ||
'--disable-features=DialMediaRouteProvider', |
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.
👌
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.
Thanks for the quick turnaround @klebba!
Fixes #167