workaround for windows to ensure console.log is always flushed before pr... #111
Conversation
| @@ -321,7 +321,9 @@ function _main(onComplete) { | |||
|
|
|||
| if (argv.help) { | |||
| optimist.showHelp(); | |||
| process.exit(0); | |||
| process.on('exit', function(){ | |||
There was a problem hiding this comment.
Interesting, hadn't heard of this before...but seems to be a KP: https://www.npmjs.org/package/exit
One thing we need to be sure of on these changes is that execution doesn't continue after exiting (which was part of the intent of those exits before). So we either need to refactor the scripts so they can return early after setting up this listener, or find a way to make sure that the remaining code doesn't continue to execute if we hit this branch in the code path.
|
What's the status of this? |
|
I think we're just waiting on the update to the PR per the inline comment. |
|
Sorry, have not had a chance to use Jest again since submitting this. I should hopefully get some time to look into this in the next week or so |
|
I'm not sure the problem with tests "continuing to execute" makes sense. If all jest tests are synchronous, then the initial thrown exception should terminate the tests anyway, so there shouldn't be any need to worry about that? @jeffmo If you want me to re-write this to use the 'exit' module I'm happy to make that change. |
|
In the code change I commented on, we want the jest CLI to stop then-and-there if |
|
Also I'm not quite sure what you meant by tests executing synchronously -- but test files themselves run asynchronously, so a failing test would not stop the Jest cli in it's tracks |
|
@jeffmo I just tried using the proposed 'exit' module instead of process.exit, and it seems to work just as intended. Both with and without the --help directive. I might not understand the problem though. |
|
@jeffmo Thanks, I think I understand now. I had assumed you meant that comment to apply to all the process.exit calls, but it makes far more sense for just the |
|
any word on when this might hit? I'd be happy to help get it done. Without this Jest doesn't provide any feedback on windows, rendering it useless as a testing tool |
|
I am happy to take this, but it's in a place where it needs some refactoring (and bin/jest.js has changed since this was written). I would go in and just finish this up myself (seems fairly straightforward), but I don't have a working Windows machine to try to port this to the latest code...so I wouldn't feel comfortable doing so without being able to verify and test. Could either @malonecj or someone else who's running on windows finish this out and confirm that it fixes things? I'm happy to take it if we can get the stop-execution problem worked out (the thing I commented inline) |
|
@jeffmo I threw something together let me know if its enough! |
|
Awesome, thanks a ton @jquense ! |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
...ocess.exit
This is a potential solution for #110
I don't have much experience with Node yet so I might be missing something here, but it at least fixed my problem and am now seeing all test results logged to console.