-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
tests(smokehouse): index for static server. print address #9541
Conversation
@@ -79,7 +79,7 @@ module.exports = [ | |||
{resourceType: 'font', requestCount: 2, size: '80000±1000'}, | |||
{resourceType: 'script', requestCount: 3, size: '55000±1000'}, | |||
{resourceType: 'image', requestCount: 2, size: '28000±1000'}, | |||
{resourceType: 'document', requestCount: 1, size: '2100±100'}, | |||
{resourceType: 'document', requestCount: 1, size: '2200±100'}, |
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.
went up b/c content-type header is now added for html docs
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 right that's easy to forget that header cost is included in these
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.
surprised it took us this long to do this, what a waste of memorizing these paths! 😆
@@ -79,6 +90,8 @@ function requestHandler(request, response) { | |||
headers['Content-Type'] = 'image/webp'; | |||
} else if (filePath.endsWith('.json')) { | |||
headers['Content-Type'] = 'application/json'; | |||
} else if (filePath.endsWith('.html') || filePath === '/') { | |||
headers['Content-Type'] = 'text/html'; |
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 feels 100% unrelated to the index part, right?
why cause conflicts for yourself in #9542 :)
const fixturePaths = glob.sync('**/*.html', {cwd: __dirname}); | ||
const html = ` | ||
<h1>Smoke test fixtures</h1> | ||
${fixturePaths.map(p => `<a href=${p}>${p}</a>`).join('<br>')} |
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.
does this work on windows, like are the paths returned by glob different?
do we care if it doesn't? probably not
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.
🤷♂
serverForOnline.listen(onlinePort, 'localhost'); | ||
serverForOffline.listen(offlinePort, 'localhost'); | ||
console.log(`online: listening on http://localhost:${onlinePort}`); | ||
console.log(`offline: listening on http://localhost:${offlinePort}`); |
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's really nothing offline or online about these, it's just how we use them for one(?) of the smoke tests (we also use them for same- and different-origin in other tests), but I'm not sure it matters what we call them :)
Co-Authored-By: Patrick Hulce <patrick.hulce@gmail.com>
Minor enhancement.