Merge pull request #2036 from strongloop/fix/ci-3.0

Fix race conditions in unit tests

test/error-handler.test.js

@@ -41,15 +41,23 @@ describe('loopback.errorHandler(options)', function() {
     //arrange
     var app = loopback();
     app.use(loopback.urlNotFound());
-    app.use(loopback.errorHandler({ includeStack: false, log: customLogger }));
+
+    var errorLogged;
+    app.use(loopback.errorHandler({
+      includeStack: false,
+      log: function customLogger(err, str, req) {
+        errorLogged = err;
+      }
+    }));
 
     //act
-    request(app).get('/url-does-not-exist').end();
+    request(app).get('/url-does-not-exist').end(function(err) {
-
+      if (err) return done(err);
       //assert
-    function customLogger(err, str, req) {
+      expect(errorLogged)
-      assert.ok(err.message === 'Cannot GET /url-does-not-exist');
+        .to.have.property('message', 'Cannot GET /url-does-not-exist');
       done();
-    }
+    });
+
   });
 });

test/replication.test.js

@@ -67,40 +67,26 @@ describe('Replication / Change APIs', function() {
     });
 
     it('should call rectifyAllChanges if no id is passed for rectifyOnDelete', function(done) {
-      SourceModel.rectifyChange = function() {
+      var calls = mockSourceModelRectify();
-        return done(new Error('Should not call rectifyChange'));
-      };
-      SourceModel.rectifyAllChanges = function() {
-        return done();
-      };
       SourceModel.destroyAll({name: 'John'}, function(err, data) {
-        if (err)
+        if (err) return done(err);
-          return done(err);
+        expect(calls).to.eql(['rectifyAllChanges']);
+        done();
       });
     });
 
     it('should call rectifyAllChanges if no id is passed for rectifyOnSave', function(done) {
-      SourceModel.rectifyChange = function() {
+      var calls = mockSourceModelRectify();
-        return done(new Error('Should not call rectifyChange'));
-      };
-      SourceModel.rectifyAllChanges = function() {
-        return done();
-      };
       var newData = {'name': 'Janie'};
       SourceModel.update({name: 'Jane'}, newData, function(err, data) {
-        if (err)
+        if (err) return done(err);
-          return done(err);
+        expect(calls).to.eql(['rectifyAllChanges']);
+        done();
       });
     });
 
     it('rectifyOnDelete for Delete should call rectifyChange instead of rectifyAllChanges', function(done) {
-      TargetModel.rectifyChange = function() {
+      var calls = mockTargetModelRectify();
-        return done();
-      };
-      TargetModel.rectifyAllChanges = function() {
-        return done(new Error('Should not call rectifyAllChanges'));
-      };
-
       async.waterfall([
         function(callback) {
           SourceModel.destroyAll({name: 'John'}, callback);
@@ -110,19 +96,14 @@ describe('Replication / Change APIs', function() {
           // replicate should call `rectifyOnSave` and then `rectifyChange` not `rectifyAllChanges` through `after save` operation
         }
       ], function(err, results) {
-        if (err)
+        if (err) return done(err);
-          return done(err);
+        expect(calls).to.eql(['rectifyChange']);
+        done();
       });
     });
 
     it('rectifyOnSave for Update should call rectifyChange instead of rectifyAllChanges', function(done) {
-      TargetModel.rectifyChange = function() {
+      var calls = mockTargetModelRectify();
-        return done();
-      };
-      TargetModel.rectifyAllChanges = function() {
-        return done(new Error('Should not call rectifyAllChanges'));
-      };
-
       var newData = {'name': 'Janie'};
       async.waterfall([
         function(callback) {
@@ -131,20 +112,16 @@ describe('Replication / Change APIs', function() {
         function(data, callback) {
           SourceModel.replicate(TargetModel, callback);
           // replicate should call `rectifyOnSave` and then `rectifyChange` not `rectifyAllChanges` through `after save` operation
-        }], function(err, result) {
+        }
-        if (err)
+      ], function(err, result) {
-          return done(err);
+        if (err) return done(err);
+        expect(calls).to.eql(['rectifyChange']);
+        done();
       });
     });
 
     it('rectifyOnSave for Create should call rectifyChange instead of rectifyAllChanges', function(done) {
-      TargetModel.rectifyChange = function() {
+      var calls = mockTargetModelRectify();
-        return done();
-      };
-      TargetModel.rectifyAllChanges = function() {
-        return done(new Error('Should not call rectifyAllChanges'));
-      };
-
       var newData = [{name: 'Janie', surname: 'Doe'}];
       async.waterfall([
         function(callback) {
@@ -155,10 +132,47 @@ describe('Replication / Change APIs', function() {
           // replicate should call `rectifyOnSave` and then `rectifyChange` not `rectifyAllChanges` through `after save` operation
         }
       ], function(err, result) {
-        if (err)
+        if (err) return done(err);
-          return done(err);
+        expect(calls).to.eql(['rectifyChange']);
+        done();
       });
     });
+
+    function mockSourceModelRectify() {
+      var calls = [];
+
+      SourceModel.rectifyChange = function(id, cb) {
+        calls.push('rectifyChange');
+        console.trace('rectifyChange');
+        process.nextTick(cb);
+      };
+
+      SourceModel.rectifyAllChanges = function(cb) {
+        calls.push('rectifyAllChanges');
+        console.trace('rectifyAllChanges');
+        process.nextTick(cb);
+      };
+
+      return calls;
+    }
+
+    function mockTargetModelRectify() {
+      var calls = [];
+
+      TargetModel.rectifyChange = function(id, cb) {
+        calls.push('rectifyChange');
+        console.trace('rectifyChange');
+        process.nextTick(cb);
+      };
+
+      TargetModel.rectifyAllChanges = function(cb) {
+        calls.push('rectifyAllChanges');
+        console.trace('rectifyAllChanges');
+        process.nextTick(cb);
+      };
+
+      return calls;
+    }
   });
 
   describe('Model.changes(since, filter, callback)', function() {