sc-forks / solidity-coverage

Code coverage for Solidity smart-contracts
MIT License
977 stars 264 forks source link

Testrpc does not exit upon coverage competion (Linux) #186

Closed adamstaveley closed 6 years ago

adamstaveley commented 6 years ago

If I run coverage twice, testrpc will throw Error: listen EADDRINUSE :::8555 on the second try.

As a result I have to manually close the testrpc process spawned by solidity-coverage before running again. It looks as though it should kill the process from a brief look through the code.

OS is Ubuntu 16.04.

cgewecke commented 6 years ago

@adamstaveley Out of curiosity how are you invoking the coverage tool? Also which version are you using?

adamstaveley commented 6 years ago

@cgewecke I have it installed locally as a dev dependency. Originally invoked through an npm script (e.g. "coverage": "solidity-coverage") but using node_modules/.bin/solidity-coverage produces the same result. I also tried reinstalling it but had the same issue.

The version is 0.4.9.

I tested on macOS today and it worked as expected with testrpc exiting on coverage completion.

area commented 6 years ago

It seems as if OSX and Linux differ on how they handle child processes when using .kill(). But we're just using childprocess.exec to directly launch testrpc, aren't we? There shouldn't be any children, should there?

fvictorio commented 6 years ago

Can confirm, this is happening to me too in Ubuntu 17.10.

mswezey23 commented 6 years ago

Confirming again - does not kill process on Ubuntu

cgewecke commented 6 years ago

It looks like Linux might require that you destroy the stdout, stdout streams of the child process as well. Will add that in the next release and see if it resolves this.

cgewecke commented 6 years ago

@adamstaveley @mswezey23 @fvictorio

Have added logic to kill stdout/stderr streams on exit to 0.5.0. If any of you have a chance, could you run that version and see if this is resolved? Tx!

zoenolan commented 6 years ago

Just updated asset-token to 0.5.0. Still seeing the node process after a coverage run on Ubuntu 17.10

mswezey23 commented 6 years ago

I still get this error as it does not close:

testRpc stout: Ganache CLI v6.1.2 (ganache-core: 2.1.0) Error: listen EADDRINUSE :::8555

fvictorio commented 6 years ago

@cgewecke Have you tried using https://www.npmjs.com/package/tree-kill?

I was writing a script that, among other things, starts ganache and I was having a hard time killing it. Using tree-kill was the only thing that worked.

cgewecke commented 6 years ago

@fvictorio Thank you! No I haven't - I looked into this more a while ago and it seemed like the shell that exec runs in is not getting killed on Linux. So tree kill looks perfect.

cgewecke commented 6 years ago

Should be fixed in 0.5.1. Thanks again @fvictorio.

Please ping if this is not the case. Tested strategy by launching testrpc-sc twice in succession in a Travis Ubuntu container here.

fvictorio commented 6 years ago

Works for me in Ubuntu 17.10. Thanks @cgewecke!

zoenolan commented 6 years ago

Also works for us on Ubuntu 17.10

mswezey23 commented 6 years ago

Works in 16.XX!

Thanks!!!