Watcher test issues in Linux

[jonathanKingston]

So the tests don't run correctly for the watcher code in Linux, I'm either getting:

• The code times out
• The code never finishes

The test in issue is this: test/cli.watcher.js

Killing the processes using the method mentioned here solves the issues:
http://krasimirtsonev.com/blog/article/Nodejs-managing-child-processes-starting-stopping-exec-spawn

However this causes the following line of the test to fail (Which I can't actually see should ever reliably pass):

      if (err && !err.killed) { throw err }

[MoOx]

So what do you propose to enhance the tests ?

[jonathanKingston]

Sorry for the delay with this; I'm going to paste the changes I made so they are not lost and a brain dump.

The regarding the following line, I know you mention you didn't write the test however I would be interested if you think a failed watcher should be killed? If that is the case then I think the code for that isn't working with Linux correctly.

      if (err && !err.killed) { throw err }

The changes I made mostly hang off just killing the listeners aggressively, however as I would like to get to the root of the problem I'm not sure the following are where cssnext should be doing:

+var psTree = require("ps-tree")
+var kill = function (pid, signal, callback) {
+  signal = signal || "SIGTERM"
+  callback = callback || function () {}
+  var killTree = true
+  if (killTree) {
+    psTree(pid, function (error, children) {
+      [pid].concat(
+        children.map(function (p) {
+          return p.PID
+        })
+      ).forEach(function (tpid) {
+        try { process.kill(tpid, signal) }
+        catch (ex) {
+          console.log(ex)
+        }
+      })
+      callback()
+    })
+  } else {
+    try { process.kill(pid, signal) }
+    catch (ex) {
+      console.log(ex)
+    }
+    callback()
+  }
+}

Killing in the name of:

-              watchImportProcess.kill()
+              kill(watchImportProcess.pid)
+              t.end()

Numbering plans:

+    t.plan(5)

Linux seemed to not wait around for file testing:

     // /**/ to get a content
     // yeah...
+    var fileTest = true
+
+    while (fileTest) {
+      try {
+        var me = fs.readFileSync(watchOut, {encoding: "utf8"})
+        if (me === "/**/") {
+          fileTest = false
+        }
+      } catch (e) {
+       // do something here
+      }
+    }

I'll await feedback and if you are ok with them, I can mop them up into a merge.

[MoOx]

This test if (err && !err.killed) { throw err } is just to allow ourselves to kill the process when tests are finished, otherwise I think the process will continue until someone kill it.

We already plan 5 tests from what I can read.

If your function helps to kill all childs, maybe it's the solution. Did you make some debug to see if we actually have other child process to kill ?

Also I am considering changing watcher because of #123, maybe that can helps to solve this issue ?

[MoOx]

up :)

[jonathanKingston]

Whoops sorry, will look into this further soon.

Yup I wasn't certain if it was actually getting killed in the watcher code (Last time I checked I couldn't see the code that did that).

Yeah certainly as I think the issue is that there is no code killing it correctly, either linux is wrong or that test is.

[MoOx]

Any updates on this ? Otherwise I will end up by closing this issue (not even sure why...)

[MoOx]

Not relevant anymore. cssnext is now postcss-cssnext. cssnext is deprecated. See postcss-cssnext migration guide http://cssnext.io/postcss/