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

test(command-dev): log dev server output on timeout #1694

Merged
merged 3 commits into from
Jan 5, 2021

Conversation

erezrokah
Copy link
Contributor

This PR reports the dev server output in our tests when we fail to spawn it.

@erezrokah erezrokah requested a review from a team as a code owner January 5, 2021 14:53
@erezrokah erezrokah requested a review from ehmicky January 5, 2021 14:53
@erezrokah erezrokah added the type: chore work needed to keep the product and development running smoothly label Jan 5, 2021
@@ -63,6 +66,9 @@ const startServer = async ({ cwd, env = {}, args = [] }) => {
})
ps.catch((error) => !selfKilled && reject(error))
})

const { timeout, ...server } = await Promise.race([serverPromise, getTimeoutPromise(SERVER_START_TIMEOUT)])
Copy link
Contributor

Choose a reason for hiding this comment

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

[sand] Do you think p-timeout might help with this pattern?
https://github.com/sindresorhus/p-timeout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 12b4af1

ps.stdout.on('data', (data) => {
if (data.toString().includes('Server now ready on')) {
ps.stdout.on('data', (dataBuffer) => {
const data = dataBuffer.toString()
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe using the encoding: 'utf-8' option of execa should provide with data string chunks instead directly. Would this work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, done in a0d8158

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: chore work needed to keep the product and development running smoothly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants