fix many test problems (#1677)

* fix many test problems

- adds [stoppable](https://npm.im/stoppable) to force-stop net & http
  servers
- upgrades to latest mocha
- much cleanup of servers
- some removal of useless code

Signed-off-by: Christopher Hiller <boneskull@boneskull.com>

* increase wait time to hack at race condition

* PoC with fork of stoppable

Signed-off-by: Christopher Hiller <boneskull@boneskull.com>

* fix custom stoppable url for newer npm

* make travis go faster; attempt to avoid npm troubles

* fix coveralls executable path

* add extra time for flake to trigger spec

Signed-off-by: Christopher Hiller <boneskull@boneskull.com>

.travis.yml

@@ -1,20 +1,10 @@
 sudo: false
 language: node_js
-env:
-  - CXX="g++-4.8"
-addons:
-  apt:
-    sources:
-    - ubuntu-toolchain-r-test
-    packages:
-    - g++-4.8
-    - gcc-4.8
 node_js:
   - "8"
   - "6"
   - "4"
 script:
-  - istanbul cover ./node_modules/.bin/grunt --report lcovonly && istanbul report text && ( cat coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js || true ) && rm -rf coverage
+  - istanbul cover ./node_modules/.bin/grunt --report lcovonly && istanbul report text && ( cat coverage/lcov.info | $(npm get prefix)/bin/coveralls || true ) && rm -rf coverage
 before_script:
-  - npm install -g istanbul
-  - npm install coveralls
+  - npm install -g istanbul coveralls

package.json

@@ -99,9 +99,10 @@
     "grunt-simple-mocha": "~0.4.1",
     "grunt-webdriver": "^2.0.3",
     "istanbul": "0.4.5",
-        "mocha": "~3.4.2",
+    "mocha": "^5.1.1",
     "should": "^8.4.0",
     "sinon": "1.17.7",
+    "stoppable": "^1.0.6",
     "supertest": "3.0.0",
     "wdio-chromedriver-service": "^0.1.1",
     "wdio-mocha-framework": "^0.5.11",

test/nodes/core/analysis/72-sentiment_spec.js

@@ -24,6 +24,10 @@ describe('sentiment Node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     afterEach(function() {
         helper.unload();
     });

test/nodes/core/core/20-inject_spec.js

@@ -24,6 +24,10 @@ describe('inject node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     afterEach(function() {
         helper.unload();
     });

test/nodes/core/core/58-debug_spec.js

@@ -25,6 +25,10 @@ describe('debug node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     beforeEach(function (done) {
         setTimeout(function() {
             done();

test/nodes/core/core/60-link_spec.js

@@ -24,6 +24,10 @@ describe('link Node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     afterEach(function() {
         helper.unload();
     });

test/nodes/core/core/80-function_spec.js

@@ -24,6 +24,10 @@ describe('function node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     afterEach(function() {
         helper.unload();
     });

test/nodes/core/core/80-template_spec.js

@@ -24,6 +24,10 @@ describe('template node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     afterEach(function() {
         helper.unload();
     });

test/nodes/core/core/89-trigger_spec.js

@@ -304,11 +304,11 @@ describe('trigger node', function() {
                     }
                     else {
                         msg.should.have.a.property("payload", "bar");
-                        spy.restore();
+                        RED.util.evaluateNodeProperty.restore();
                         done();
                     }
                 }
-                catch(err) { spy.restore(); done(err); }
+                catch(err) { RED.util.evaluateNodeProperty.restore(); done(err); }
             });
             n1.emit("input", {payload:null});
         });
@@ -384,6 +384,7 @@ describe('trigger node', function() {
     });
 
     it('should be able to extend the delay', function(done) {
+        this.timeout(5000); // add extra time for flake
         var spy = sinon.stub(RED.util, 'evaluateNodeProperty',
             function(arg1, arg2, arg3, arg4) { return arg1; }
         );

test/nodes/core/io/21-httprequest_spec.js

@@ -19,6 +19,7 @@ var http = require("http");
 var should = require("should");
 var express = require("express");
 var bodyParser = require('body-parser');
+var stoppable = require('stoppable');
 var helper = require("../../helper.js");
 var httpRequestNode = require("../../../../nodes/core/io/21-httprequest.js");
 var hashSum = require("hash-sum");
@@ -30,13 +31,10 @@ describe('HTTP Request Node', function() {
 
     function startServer(done) {
         testPort += 1;
-        testServer = http.createServer(testApp);
-        testServer.on('error', function(err) {
-            startServer(done);
-        });
+        testServer = stoppable(http.createServer(testApp));
         testServer.listen(testPort,function(err) {
-            done();
-        })
+            done(err);
+        });
     }
 
     function getTestURL(url) {
@@ -57,13 +55,18 @@ describe('HTTP Request Node', function() {
             }
             res.json(result);
         });
-        startServer(function() {
+        startServer(function(err) {
+            if (err) {
+                done(err);
+            }
             helper.startServer(done);
         });
     });
 
-    after(function() {
-        testServer.close();
+    after(function(done) {
+        testServer.stop(() => {
+            helper.stopServer(done);
+        });
     });
     afterEach(function() {
         helper.unload();

test/nodes/core/io/22-websocket_spec.js

@@ -61,6 +61,10 @@ describe('websocket Node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     afterEach(function() {
         closeAll();
         helper.unload();

test/nodes/core/io/31-tcpin_spec.js

@@ -16,6 +16,7 @@
 
 var net = require("net");
 var should = require("should");
+var stoppable = require('stoppable');
 var helper = require("../../helper.js");
 var tcpinNode = require("../../../../nodes/core/io/31-tcpin.js");
 
@@ -26,20 +27,13 @@ describe('TCP in Node', function() {
     var server_port = 9300;
     var reply_data = undefined;
 
-    before(function(done) {
-        done();
-    });
-
-    after(function() {
-    });
-
     beforeEach(function(done) {
         startServer(done);
     });
 
-    afterEach(function() {
-        stopServer();
+    afterEach(function(done) {
         helper.unload();
+        stopServer(done);
     });
 
     function sendArray(sock, array) {
@@ -55,15 +49,15 @@ describe('TCP in Node', function() {
     
     function startServer(done) {
         server_port += 1;
-        server = net.createServer(function(c) {
+        server = stoppable(net.createServer(function(c) {
             sendArray(c, reply_data);
-        }).listen(server_port, "localhost", function(err) {
+        })).listen(server_port, "localhost", function(err) {
             done(err);
         });
     }
 
-    function stopServer() {
-        server.close();
+    function stopServer(done) {
+        server.stop(done);
     }
     
     function send(wdata) {

test/nodes/core/io/31-tcprequest_spec.js

@@ -16,6 +16,7 @@
 
 var net = require("net");
 var should = require("should");
+var stoppable = require('stoppable');
 var helper = require("../../helper.js");
 var tcpinNode = require("../../../../nodes/core/io/31-tcpin.js");
 
@@ -26,7 +27,7 @@ describe('TCP Request Node', function() {
 
     function startServer(done) {
         port += 1;
-        server = net.createServer(function(c) {
+        server = stoppable(net.createServer(function(c) {
 	    c.on('data', function(data) {
 		var rdata = "ACK:"+data.toString();
 		c.write(rdata);
@@ -34,7 +35,7 @@ describe('TCP Request Node', function() {
             c.on('error', function(err) {
 		startServer(done);
             });
-        }).listen(port, "127.0.0.1", function(err) {
+        })).listen(port, "127.0.0.1", function(err) {
             done();
         });
     }
@@ -43,8 +44,8 @@ describe('TCP Request Node', function() {
         startServer(done);
     });
 
-    after(function() {
-        server.close();
+    after(function(done) {
+        server.stop(done);
     });
 
     afterEach(function() {

test/nodes/core/io/32-udpin_spec.js

@@ -27,7 +27,8 @@ describe('UDP in Node', function() {
         helper.startServer(done);
     });
 
-    after(function() {
+    after(function(done) {
+        helper.stopServer(done);
     });
 
     afterEach(function() {

test/nodes/core/io/32-udpout_spec.js

@@ -27,7 +27,8 @@ describe('UDP out Node', function() {
         helper.startServer(done);
     });
 
-    after(function() {
+    after(function(done) {
+        helper.stopServer(done);
     });
 
     afterEach(function() {
@@ -38,7 +39,7 @@ describe('UDP out Node', function() {
         var sock = dgram.createSocket('udp4');
         sock.on('message', function(msg, rinfo) {
             msg.should.deepEqual(data);
-            done();
+            sock.close(done);
         });
         sock.bind(port, '127.0.0.1');
         port++;

test/nodes/core/logic/10-switch_spec.js

@@ -28,8 +28,8 @@ describe('switch Node', function() {
 
     afterEach(function(done) {
         helper.unload();
-        helper.stopServer(done);
         RED.settings.nodeMessageBufferMaxLength = 0;
+        helper.stopServer(done);
     });
 
     it('should be loaded with some defaults', function(done) {

test/nodes/core/logic/15-change_spec.js

@@ -578,7 +578,6 @@ describe('change Node', function() {
         });
 
         it('reports invalid regex', function(done) {
-            var sinon = require('sinon');
             var flow = [{"id":"changeNode1","type":"change","action":"change","property":"payload","from":"\\+**+","to":"NUMBER","reg":true,"name":"changeNode","wires":[["helperNode1"]]},
                         {id:"helperNode1", type:"helper", wires:[]}];
             helper.load(changeNode, flow, function() {

test/nodes/core/logic/16-range_spec.js

@@ -135,11 +135,13 @@ describe('range Node', function() {
             var sinon = require('sinon');
             sinon.stub(rangeNode1, 'log', function(log) {
                 if (log.indexOf("notnumber") > -1) {
+                    rangeNode1.log.restore();
                     done();
                 } else {
                     try {
                         should.fail(null, null, "Non-number inputs should be reported!");
                     } catch (err) {
+                        rangeNode1.log.restore();
                         done(err);
                     }
                 }

test/nodes/core/logic/17-split_spec.js

@@ -26,6 +26,10 @@ describe('SPLIT node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     afterEach(function() {
         helper.unload();
     });
@@ -268,6 +272,10 @@ describe('JOIN node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     afterEach(function() {
         helper.unload();
         RED.settings.nodeMessageBufferMaxLength = 0;

test/nodes/core/logic/18-sort_spec.js

@@ -25,6 +25,10 @@ describe('SORT node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     afterEach(function() {
         helper.unload();
         RED.settings.nodeMessageBufferMaxLength = 0;

test/nodes/core/logic/19-batch_spec.js

@@ -26,6 +26,10 @@ describe('BATCH node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     afterEach(function() {
         helper.unload();
         RED.settings.nodeMessageBufferMaxLength = 0;

test/nodes/core/parsers/70-CSV_spec.js

@@ -24,6 +24,10 @@ describe('CSV node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     afterEach(function() {
         helper.unload();
     });

test/nodes/core/parsers/70-HTML_spec.js

@@ -15,7 +15,6 @@
  **/
 
 var should = require("should");
-var sinon = require("sinon");
 var path = require("path");
 var fs = require('fs-extra');
 
@@ -31,6 +30,10 @@ describe('html node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     beforeEach(function() {
         fs.existsSync(file).should.be.true();
     });

test/nodes/core/parsers/70-JSON_spec.js

@@ -24,6 +24,10 @@ describe('JSON node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     afterEach(function() {
         helper.unload();
     });

test/nodes/core/parsers/70-XML_spec.js

@@ -24,6 +24,10 @@ describe('XML node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     afterEach(function() {
         helper.unload();
     });

test/nodes/core/parsers/70-YAML_spec.js

@@ -24,6 +24,10 @@ describe('YAML node', function() {
         helper.startServer(done);
     });
 
+    after(function(done) {
+        helper.stopServer(done);
+    });
+
     afterEach(function() {
         helper.unload();
     });

test/nodes/core/storage/28-tail_spec.js

@@ -130,15 +130,20 @@ describe('tail Node', function() {
         });
     }
 
-    it('should throw an error if run on Windows', function(done) {
+    it('should throw an error if run on Windows', function() {
         // Stub os platform so we can make it look like windows
         var os = require('os');
         var spy = sinon.stub(os, 'platform', function(arg) { return("windows"); });
 
         /*jshint immed: false */
+        try {
             (function() { tailNode("1234"); }).should.throw();
+        } catch (err) {
+            throw err;
+        }
+        finally {
             os.platform.restore();
-        done();
+        }
     });
 
     /*

test/nodes/core/storage/50-file_spec.js

@@ -28,7 +28,7 @@ describe('file Nodes', function() {
 
         var resourcesDir = path.join(__dirname,"..","..","..","resources");
         var fileToTest = path.join(resourcesDir,"50-file-test-file.txt");
-        var wait = 150;
+        var wait = 250;
 
         beforeEach(function(done) {
             //fs.writeFileSync(fileToTest, "File message line 1\File message line 2\n");

test/nodes/helper.js

@@ -19,6 +19,7 @@ var sinon = require("sinon");
 var when = require("when");
 var request = require('supertest');
 var express = require("express");
+var stoppable = require('stoppable');
 var nock;
 if (!process.version.match(/^v0\.[0-9]\./)) {
     // only set nock for node >= 0.10
@@ -132,7 +133,7 @@ module.exports = {
     },
 
     startServer: function(done) {
-        server = http.createServer(function(req,res) { app(req,res); });
+        server = stoppable(http.createServer(function(req,res) { app(req,res); }), 0);
         RED.init(server, {
             SKIP_BUILD_CHECK: true,
             logging:{console:{level:'off'}}
@@ -150,13 +151,13 @@ module.exports = {
     stopServer: function(done) {
         if (server) {
             try {
-                server.on('close', function() {
                 comms.stop();
-                });
-                server.close(done);
+                server.stop(done);
             } catch(e) {
                 done();
             }
+        } else {
+            done();
         }
     },
 

test/red/api/admin/nodes_spec.js

@@ -813,7 +813,6 @@ describe("api/admin/nodes", function() {
 
     describe('get icons', function() {
         it('returns icon list', function(done) {
-            debugger;
             initNodes({
                 nodes:{
                     getNodeIcons: function() {

test/red/api/editor/comms_spec.js

@@ -16,6 +16,7 @@
 
 var should = require("should");
 var sinon = require("sinon");
+const stoppable = require('stoppable');
 
 var when = require("when");
 var http = require('http');
@@ -30,13 +31,8 @@ var Tokens = require("../../../../red/api/auth/tokens");
 var address = '127.0.0.1';
 var listenPort = 0; // use ephemeral port
 
-describe("api/editor/comms", function() {
 
-    beforeEach(function (done) {
-        setTimeout(function() {
-            done();
-        }, 55);
-    });
+describe("api/editor/comms", function() {
 
     describe("with default keepalive", function() {
         var server;
@@ -44,7 +40,7 @@ describe("api/editor/comms", function() {
         var port;
         before(function(done) {
             sinon.stub(Users,"default",function() { return when.resolve(null);});
-            server = http.createServer(function(req,res){app(req,res)});
+            server = stoppable(http.createServer(function(req,res){app(req,res)}));
             comms.init(server, {
                 settings:{},
                 log:{warn:function(){},_:function(){},trace:function(){},audit:function(){}},
@@ -59,9 +55,10 @@ describe("api/editor/comms", function() {
             });
         });
 
-        after(function() {
+        after(function(done) {
             Users.default.restore();
             comms.stop();
+            server.stop(done);
         });
 
         it('accepts connection', function(done) {
@@ -150,7 +147,7 @@ describe("api/editor/comms", function() {
             });
         });
 
-        it.skip('listens for node/status events');
+        it('listens for node/status events');
     });
     describe("disabled editor", function() {
         var server;
@@ -158,7 +155,7 @@ describe("api/editor/comms", function() {
         var port;
         before(function(done) {
             sinon.stub(Users,"default",function() { return when.resolve(null);});
-            server = http.createServer(function(req,res){app(req,res)});
+            server = stoppable(http.createServer(function(req,res){app(req,res)}));
             comms.init(server, {
                 settings:{disableEditor:true},
                 log:{warn:function(){},_:function(){},trace:function(){},audit:function(){}},
@@ -173,9 +170,10 @@ describe("api/editor/comms", function() {
             });
         });
 
-        after(function() {
+        after(function(done) {
             Users.default.restore();
             comms.stop();
+            server.stop(done);
         });
 
         it('rejects websocket connections',function(done) {
@@ -197,7 +195,7 @@ describe("api/editor/comms", function() {
         var port;
         before(function(done) {
             sinon.stub(Users,"default",function() { return when.resolve(null);});
-            server = http.createServer(function(req,res){app(req,res)});
+            server = stoppable(http.createServer(function(req,res){app(req,res)}));
             comms.init(server, {
                 settings:{httpAdminRoot:"/adminPath"},
                 log:{warn:function(){},_:function(){},trace:function(){},audit:function(){}},
@@ -212,9 +210,10 @@ describe("api/editor/comms", function() {
             });
         });
 
-        after(function() {
+        after(function(done) {
             Users.default.restore();
             comms.stop();
+            server.stop(done);
         });
 
         it('accepts connections',function(done) {
@@ -236,7 +235,7 @@ describe("api/editor/comms", function() {
         var port;
         before(function(done) {
             sinon.stub(Users,"default",function() { return when.resolve(null);});
-            server = http.createServer(function(req,res){app(req,res)});
+            server = stoppable(http.createServer(function(req,res){app(req,res)}));
             comms.init(server,{
                 settings:{httpAdminRoot:"/adminPath"},
                 log:{warn:function(){},_:function(){},trace:function(){},audit:function(){}},
@@ -251,9 +250,10 @@ describe("api/editor/comms", function() {
             });
         });
 
-        after(function() {
+        after(function(done) {
             Users.default.restore();
             comms.stop();
+            server.stop(done);
         });
 
         it('accepts connections',function(done) {
@@ -275,7 +275,7 @@ describe("api/editor/comms", function() {
         var port;
         before(function(done) {
             sinon.stub(Users,"default",function() { return when.resolve(null);});
-            server = http.createServer(function(req,res){app(req,res)});
+            server = stoppable(http.createServer(function(req,res){app(req,res)}));
             comms.init(server, {
                 settings:{httpAdminRoot:"adminPath"},
                 log:{warn:function(){},_:function(){},trace:function(){},audit:function(){}},
@@ -290,9 +290,10 @@ describe("api/editor/comms", function() {
             });
         });
 
-        after(function() {
+        after(function(done) {
             Users.default.restore();
             comms.stop();
+            server.stop(done);
         });
 
         it('accepts connections',function(done) {
@@ -314,7 +315,7 @@ describe("api/editor/comms", function() {
         var port;
         before(function(done) {
             sinon.stub(Users,"default",function() { return when.resolve(null);});
-            server = http.createServer(function(req,res){app(req,res)});
+            server = stoppable(http.createServer(function(req,res){app(req,res)}));
             comms.init(server, {
                 settings:{webSocketKeepAliveTime: 100},
                 log:{warn:function(){},_:function(){},trace:function(){},audit:function(){}},
@@ -328,9 +329,10 @@ describe("api/editor/comms", function() {
                 done();
             });
         });
-        after(function() {
+        after(function(done) {
             Users.default.restore();
             comms.stop();
+            server.stop(done);
         });
         it('are sent', function(done) {
             var ws = new WebSocket(url);
@@ -400,7 +402,7 @@ describe("api/editor/comms", function() {
             });
 
 
-            server = http.createServer(function(req,res){app(req,res)});
+            server = stoppable(http.createServer(function(req,res){app(req,res)}));
             comms.init(server,{
                 settings:{adminAuth:{}},
                 log:{warn:function(){},_:function(){},trace:function(){},audit:function(){}},
@@ -414,11 +416,12 @@ describe("api/editor/comms", function() {
                 done();
             });
         });
-        after(function() {
+        after(function(done) {
             getDefaultUser.restore();
             getUser.restore();
             getToken.restore();
             comms.stop();
+            server.stop(done);
         });
 
         it('prevents connections that do not authenticate',function(done) {
@@ -490,7 +493,7 @@ describe("api/editor/comms", function() {
         var getDefaultUser;
         before(function(done) {
             getDefaultUser = sinon.stub(Users,"default",function() { return when.resolve({permissions:"read"});});
-            server = http.createServer(function(req,res){app(req,res)});
+            server = stoppable(http.createServer(function(req,res){app(req,res)}));
             comms.init(server, {
                 settings:{adminAuth:{}},
                 log:{warn:function(){},_:function(){},trace:function(){},audit:function(){}},
@@ -504,9 +507,10 @@ describe("api/editor/comms", function() {
                 done();
             });
         });
-        after(function() {
+        after(function(done) {
             getDefaultUser.restore();
             comms.stop();
+            server.stop(done);
         });
 
         it('allows anonymous connections that do not authenticate',function(done) {

test/red/api/editor/sshkeys_spec.js

@@ -52,9 +52,9 @@ describe("api/editor/sshkeys", function() {
                     session_data = _session;
                     return when.resolve();
                 }
+            }
         },
-            log:{audit:function(){},error:function(msg){errors.push(msg)}}
-        },
+        log:{audit:function(){},error:function(msg){errors.push(msg)},trace:function(){}},
         storage: {
             projects: {
                 ssh: {

test/red/runtime/storage/localfilesystem/projects/git/authServer_spec.js

@@ -49,7 +49,11 @@ describe("localfilesystem/projects/git/authServer", function() {
                 return sendPrompt(rs.path,"Password");
             }).then(function(response) {
                 response.should.eql("TEST_PASS");
-            }).then(done).catch(function(err) {
+            }).then(() => {
+                rs.close();
+                done();
+            }).catch(function(err) {
+                rs.close();
                 done(err);
             })
 
@@ -64,7 +68,11 @@ describe("localfilesystem/projects/git/authServer", function() {
                 return sendPrompt(rs.path,"Enter");
             }).then(function(response) {
                 response.should.eql("TEST_PASSPHRASE");
-            }).then(done).catch(function(err) {
+            }).then(() => {
+                rs.close();
+                done();
+            }).catch(function(err) {
+                rs.close();
                 done(err);
             })