-
-
Notifications
You must be signed in to change notification settings - Fork 214
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
Remove exit handler on exit #129
Conversation
Would you be able to add a test for this? |
test.js
Outdated
const listener = ee.listeners('exit').pop(); | ||
|
||
await new Promise((resolve, reject) => { | ||
child.on('error', reject); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong indentation.
test.js
Outdated
}); | ||
|
||
const included = ee.listeners('exit').indexOf(listener) !== -1; | ||
t.false(included); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
t.false(ee.listeners('exit').includes(listener));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
includes
is not supported by Node.js 4.
Putting the expression in t.false
produces a long and unhelpful message on test failure:
/Users/rberdeen/work/execa/test.js:524
523:
524: t.false(ee.listeners('exit').indexOf(listener) !== -1);
525: });
Value is not `false`:
true
ee.listeners('exit').indexOf(listener) !== -1
=> true
-1
=> -1
ee.listeners('exit').indexOf(listener)
=> 15
listener
=> Function {}
ee.listeners('exit')
=> [
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
]
ee
=> EventEmitter {
_events: {
afterexit: Function {},
exit: [
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
Function {},
],
},
_eventsCount: 2,
_maxListeners: Infinity,
count: 2,
domain: null,
emitted: {},
infinite: true,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right. Array.includes
isn't handled by babel
(in AVA).
Thank you :) |
Always call
removeExitHandler()
when the spawned process exits, not only if.then()
is called.There was already the
cleanupTimeout()
function that was called whenever the spawned process exited, so I repurposed that to clean up everything.Fixes #128