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

Kill the Flow server after coverage report generation #153

Open
tibdex opened this issue May 18, 2018 · 2 comments
Open

Kill the Flow server after coverage report generation #153

tibdex opened this issue May 18, 2018 · 2 comments

Comments

@tibdex
Copy link

tibdex commented May 18, 2018

flow-coverage-report starts a Flow server in:

const res = await exec(`${flowCommandPath} status --json`,
{cwd: projectDir, maxBuffer: Infinity},
{dontReject: true});

but never stops it. The server process thus continues running unnoticed in the background.

When using services like CircleCI, running flow-coverage-report without running flow stop manually afterwards will fail the build. Indeed, CircleCI will consider that the Flow process has been running for too long, will terminate it, and mark the build as failed.

Would you please consider making flow-coverage-report stop the Flow server when it started it itself?

Thanks.

@rpl
Copy link
Owner

rpl commented Jun 19, 2018

@tibdex when running on a developer machine, the flow server is likely to be shared by one started by editor plugins that integrates flow as well, and killing the server could make the editor plugins to complain (and the developer may be wondering why, because he/she has never started the flow server manually and it is not clear what has stopped it) and generating flow coverage reports more than ones while developing is likely to become slower (because the flow server will need to be restarted and all the knowledge of the flow types and their coverage has to be rebuilt from scratch).

How would sound to you if flow-coverage-report would stop the flow server only when a certain environment var is defined?
(e.g. CI_FLOW_STOP, it should be good enough for the CI scenario and, given that the user doesn't probably need to call it so often, a command line option may not be really necessary).

@tibdex
Copy link
Author

tibdex commented Jun 19, 2018

Hey Luca, thank you for your answer.

I agree that we don't want to stop the Flow server if it's shared. That's why I said "stop the Flow server when [flow-coverage-report] started it itself" because then it very likely means that the Flow server isn't shared.
Indeed, we could expect the Flow server to be started by the user's editor integration or directly by the user running npm run flow or yarn run flow before flow-coverage-report is actually used. Do you agree?
So, if there is a way to detect that flow-coverage-report is responsible for having started the currently running Flow server, I think we should use it to stop the server at the end of the coverage report generation. If you think it's too complicated/fragile, then sure the environment variable would be a nice workaround ;)

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants