Conversation
Codecov Report
@@ Coverage Diff @@
## master #3129 +/- ##
=======================================
Coverage 97.95% 97.95%
=======================================
Files 207 207
Lines 7667 7667
Branches 1728 1728
=======================================
Hits 7510 7510
Misses 157 157 Continue to review full report at Codecov.
|
|
Yes, I think this has not worked right for some time. Prior to this change I see this in the console when I quit. Now I see this. Which still seems slightly broken? |
To recap, when running
Since all of these processes belong to the same process group (PGID 5712 here) pressing Ctrl+C will send SIGINT to all of them. Are you saying that the node process running |
|
I don't experience orphan process when I use
After the Based on this, it seems that the gulp process handles the SIGINT. |
|
The way to test this is to make create a syntax error (by removing a bracket or parenthesis) while running |
esanzgar
force-pushed
the
kill-karma
branch
2 times, most recently
from
545b8bf to
0517409
Compare
|
I present a modified solution that doesn't leave: |
|
This works for me, and I would give this my approval, but please check with Rob too. |
|
Eduardo's comments above provide some understanding of what is going on but I didn't feel we'd gotten to the bottom of it. With some further experimentation I have a minimal reproduction.
diff --git a/gulpfile.js b/gulpfile.js
index c16ab5292..66fea7833 100644
--- a/gulpfile.js
+++ b/gulpfile.js
@@ -354,6 +354,16 @@ function runKarma(done) {
).start();
}
+gulp.task('longtask', done => {
+ setTimeout(() => {
+ done();
+ }, 20000);
+});
+
+process.on('SIGINT', () => {
+ console.log('gulp process received sigint');
+});
+
// Unit and integration testing tasks.
// Some (eg. a11y) tests rely on CSS bundles, so build these first.
gulp.task(
So it looks like what is happening here is that if a process does not register a SIGINT handler, the default one kills the gulp process. If it does register a SIGINT handler, then the handler has to do that itself. My guess then as to what happens with Karma is that Karma is installing a SIGINT handler of its own, and sure enough this is what is happening. Experimenting locally with interrupting Karma at different points I found that:
So it seems that depending on when Karma's SIGINT handler receives the signal, it will always call the On that basis, what I think we want to do here is to give Karma a small amount of time to finish running and cleanup when process.on('SIGINT', () => {
// Give Karma a chance to handle SIGINT and cleanup, but forcibly
// exit if it takes too long.
setTimeout(() => process.exit(1), 5000);
});Thoughts? |
|
I tried Robert's suggestion: but I got an orphaned karma process in this scenario:
If the setTimeout is removed then I got no orphaned process. Based on this empirical test, I would suggest we keep the patch proposed in this PR. |
The only way to kill `yarn test --watch` is sending a SIGINT via a <kbd>Ctrl</kbd>+<kbd>C</kbd> keyboard shorcut. Killing karma process in this way sometimes leaves the parent gulp process orphan. That's because when karma is killed by SIGINT sometimes doesn't run the `done` in the callback, hence leaving the gulp process waiting for the `done` signal. On more rare occasions, I have seen orphan karma process too. The solution presented here registers a listener for SIGINT and call the `done` function. It doesn't unregister the event.
|
@robertknight your solution works, so I am pushing your solution. |
robertknight
left a comment
robertknight
left a comment
There was a problem hiding this comment.
I'm happy to get this merged to fix the problem when running tests through yarn test specifically, as opposed to gulp test. However it would be good to not have to copy this code into each project where we are using a combination of yarn + gulp + Karma. Perhaps for example we could fix this issue upstream in Karma itself.
3 participants
The only way to kill
yarn test --watchis sending a SIGINT via aCtrl+C keyboard shorcut. Killing karma process in
this way sometimes leaves the parent gulp process orphan. That's because
when karma is killed by SIGINT sometimes doesn't run the
donein thecallback, hence leaving the gulp process waiting for the
donesignal.On more rare occasions, I have seen orphan karma process too.
The solution presented here registers a listener for SIGINT and call the
donefunction. It doesn't unregister the event.