fix kill and make it work better on more platforms

lib/kill.mjs

@@ -4,25 +4,41 @@ import { spawn, exec } from 'child_process'
 const execPromise = promisify(exec)
 
 export default function kill(pid, signal) {
-  let pids = new Set()
+  let pids = new Set([pid])
   let getSpawn = null
+  let getPids = null
 
   switch (process.platform) {
     case 'win32':
-      return execPromise('taskkill /pid ' + pid + ' /T /F')
+      return execPromise('taskkill /pid ' + pid + ' /T /F').then(() => pids)
     case 'darwin':
       getSpawn = function(parentPid) {
         return spawn('pgrep', ['-P', parentPid])
       }
+      getPids = function(data) {
+        return data.match(/\d+/g).map(Number)
+      }
       break
     default:
       getSpawn = function (parentPid) {
-        return spawn('ps', ['-o', 'pid', '--no-headers', '--ppid', parentPid])
+        return exec('ps -opid="" -oppid="" | grep ' + parentPid)
+      }
+      getPids = function(data, parentPid) {
+        let output = data.trim().split('\n')
+        return output.map(line => {
+          let [child, parent] = line.trim().split(/ +/)
+
+          if (Number(parent) === parentPid) {
+            return Number(child)
+          }
+
+          return 0
+        }).filter(x => x)
       }
       break
   }
 
-  return buildTree(pids, getSpawn, pid)
+  return buildTree(pids, getSpawn, getPids, pid)
     .then(function() {
       for (let pid of pids) {
         try {
@@ -31,28 +47,38 @@ export default function kill(pid, signal) {
           if (err.code !== 'ESRCH') throw err;
         }
       }
+      return pids
     })
 }
 
-function buildTree(allPids, spawnGetChildren, parentPid) {
+function buildTree(allPids, spawnGetChildren, spawnGetPids, parentPid) {
   allPids.add(parentPid)
 
   let ps = spawnGetChildren(parentPid)
   let data = ''
+  let err = ''
   ps.stdout.on('data', function(buf) {
     data += buf.toString('ascii')
   })
+  ps.stderr.on('data', function(buf) {
+    err += buf.toString('ascii')
+  })
 
-  return new Promise(function(res) {
+  return new Promise(function(res, rej) {
     ps.on('close', function(code) {
-      // Check if we got error code (usually means empty results)
+      // Check if ps errored out
-      if (code !== 0) return res()
+      if (code !== 0 && err.trim()) {
+        return rej(new Error('Error running ps to kill processes:\n\t' + err))
+      }
+
+      // Check if we otherwise got an error code (usually means empty results)
+      if (code !== 0 || !data.trim()) return res()
+
+      let pids = spawnGetPids(data, parentPid)
 
       res(Promise.all(
-        allData.match(/\d+/g)
+        pids.filter(pid => pid && !allPids.has(pid))
-          .map(Number)
+          .map(buildTree.bind(this, allPids, spawnGetChildren, spawnGetPids))
-          .filter(pid => !bla.has(pid))
-          .map(buildTree.bind(this, allPids, spawnGetChildren))
       ))
     })
   })

package.json

@@ -1,6 +1,6 @@
 {
   "name": "eltro",
-  "version": "1.4.4",
+  "version": "1.4.5",
   "description": "Eltro is a tiny no-dependancy test framework for node",
   "main": "index.mjs",
   "scripts": {

test/eltro.test.mjs

@@ -234,7 +234,6 @@ e.test('Eltro should support capturing unknown errors outside scope', async func
   t.describe('', function() {
     t.test('', function(cb) {
       process.nextTick(function() {
-        console.log('throw')
         throw assertError
       })
     })

test/kill/kill.test.mjs

@@ -1,4 +1,4 @@
-import { fork } from 'child_process'
+import { spawn } from 'child_process'
 import t from '../../lib/eltro.mjs'
 import assert from '../../lib/assert.mjs'
 import kill from '../../lib/kill.mjs'
@@ -13,7 +13,7 @@ t.describe('kill', function() {
   })
 
   t.test('should kill process correctly', function(done) {
-    worker = fork('./test/')
+    worker = spawn('node', ['./test/kill/runner.mjs'])
     assert.ok(worker.pid)
 
     worker.on('exit', done.finish(function(code, signal) {
@@ -22,4 +22,20 @@ t.describe('kill', function() {
 
     kill(worker.pid)
   })
+
+  t.test('should succeed in killing tree', async function() {
+    worker = spawn('node', ['./test/kill/runner.mjs'])
+    assert.ok(worker.pid)
+
+    // Give it some time to start
+    await new Promise(res => {
+      worker.stdout.on('data', function(data) {
+        if (data.toString().indexOf('secondary') >= 0) res()
+      })
+    })
+
+    return kill(worker.pid).then(function(pids) {
+      assert.strictEqual(pids.size, 2)
+    })
+  })
 })

test/kill/runner.mjs

@@ -1 +1,11 @@
-setInterval(function() {}, 1000)
+import { spawn } from 'child_process'
+
+console.log('primary', process.pid)
+
+let secondary = spawn('node', ['./test/kill/second_runner.mjs'])
+
+secondary.stdout.on('data', function(data) {
+  process.stdout.write(data)
+})
+
+setInterval(function() { console.log('primary', process.pid) }, 100)

test/kill/second_runner.mjs

@@ -0,0 +1,2 @@
+console.log('secondary', process.pid)
+setInterval(function() { console.log('secondary', process.pid) }, 100)