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

Do not attempt to start the server when usedPortAction is ignore and isPortTaken is true #219

Merged
merged 1 commit into from
Apr 3, 2019
Merged

Conversation

brandonaaron
Copy link
Contributor

Summary

I have a dev server already running and used usedPortAction: 'ignore' but the server is still attempted to be started despite the port showing as being taken. This patch puts a fork in the code to not attempt to start the server if ignore is used and isPortTaken is true.

Test plan

Start a server on a given port. Then attempt to use the same port in jest-dev-server config using usedPortAction: 'ignore'. Without the patch, the code will throw an EADDRINUSE error. This is because it will still attempt to run the given command. With the patch, it will simply run the tests.

Workaround

For now, I'm just trapping the EADDRINUSE error for my command (in this specific case it is an express.js server):

  command: `node -e "server = require('./server');server.listen(${config.get('ports.server')}).on('error', (err) => { if (err.code !== 'EADDRINUSE') throw new Error(err) })"

@gregberge
Copy link
Member

Hello @brandonaaron, thanks for this, I think it is great but it adds a lot of complexity in the code. @xiaoyuhen do you have ideas to refactor it?

@xiaoyuhen
Copy link
Contributor

😢I can't think of a better solution, maybe and some comments will be easy to understand.

@gregberge
Copy link
Member

@xiaoyuhen OK then, we can merge it.

@gregberge gregberge merged commit 7df3721 into argos-ci:master Apr 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants