git2consul - npm Package Compare versions

Comparing version 0.7.1 to 0.7.2

lib/consul/index.js

@@ -65,3 +65,3 @@ var fs = require('fs');
   logger.trace('Deleting key %s', key_name);
-  
+
   consul.kv.del({'key': key_name, token: token}, function(err) {
@@ -68,0 +68,0 @@ /* istanbul ignore if */

lib/git/hooks/webhook.js

@@ -96,36 +96,21 @@ var util = require('util');
 
-        var res_sent = false;
-
         for (var i=0; i<changes.length; ++i) {
           var change = changes[i];
           var to_hash = change.to_hash;
-          logger.debug('Webhook noted change to branch %s with commit id %s', change.branch, change.to_hash);
+          logger.info('Webhook noted change to branch %s with commit id %s', change.branch, change.to_hash);
 
           // Update consul git branch
           var branch = repo.branches[change.branch];
-          if (!branch) {
-            logger.trace('Not tracking branch %s, ignoring.', change.branch);
-            // Only return a response for the first change handled by this webhook.
-            if (res_sent) {
-              return;
-            }
-            res_sent = true;
-            return res.send('ok');
+          if (branch) {
+            branch.handleRefChange(change.to_hash, function(err) {
+              /* istanbul ignore next */
+              if (err) logger.error(err);
+
+              logger.debug('Updates in branch %s complete', change.branch);
+            });
           }
-          branch.handleRefChange(change.to_hash, function(err) {
-            /* istanbul ignore next */
-            if (err) logger.error(err);
-
-            logger.debug('Updates in branch %s complete', change.branch);
-            // Only return a response for the first change handled by this webhook.
-            if (res_sent) {
-              return;
-            }
-            res_sent = true;
-            return res.send('ok');
-          });
         }
-      } else {
-        res.send('ok');
       }
+
+      res.send('ok');
     });
@@ -132,0 +117,0 @@ });

lib/git/repo.js

@@ -71,3 +71,2 @@ var _ = require('underscore');
     --pending_operations;
-    logger.trace("There are now %s pending ops", pending_operations);
     if (pending_operations === 0) return cb(errs.length > 0 ? errs : undefined);
@@ -74,0 +73,0 @@ };

package.json

 {
   "name": "git2consul",
   "description": "System for moving data from git to consul",
-  "version": "0.7.1",
+  "version": "0.7.2",
   "contributors": [
@@ -6,0 +6,0 @@ {

test/git2consul_init_test.js

@@ -170,10 +170,3 @@ var should = require('should');
       branch = repo.branches['master'];
-
-      // Handle the initial sync of this repo.  Init adds a file to the remote repo, and this line syncs
-      // that to our local cache and to consul.
-      // TODO: Is this necessary?
-      branch.handleRefChange(0, function(err) {
-        if (err) return done(err);
-        done();
-      });
+      done();
     });
@@ -180,0 +173,0 @@ });

test/git2consul_key_name_test.js

@@ -25,9 +25,3 @@ var should = require('should');
       branch = repo.branches['master'];
-
-      // Handle the initial sync of this repo.  Init adds a file to the remote repo, and this line syncs
-      // that to our local cache and to consul.
-      branch.handleRefChange(0, function(err) {
-        if (err) return done(err);
-        done();
-      });
+      done();
     });
@@ -34,0 +28,0 @@ });

test/git2consul_webhook_and_polling_test.js

@@ -77,2 +77,10 @@ var should = require('should');
 
+// Give us a mechanism to report on which set of hook params we're testing.
+var test_counter = 0;
+var test_descriptions = [
+  "valid updates",
+  "changes to untracked branches",
+  "changes to non-HEAD refs"    
+];
+
 [
@@ -90,3 +98,3 @@ // Test webhooks sharing a port
     'port': 5252,
-    'body': { refChanges: [{refId: "refs/heads/master", toHash: "0"}]},
+    'body': { refChanges: [{refId: "refs/heads/master", toHash: "0"}, {refId: "refs/heads/master", toHash: "0"}] },
     'fqurl': 'http://localhost:5252/stashpoke'
@@ -113,3 +121,3 @@ },{
     'port': 5253,
-    'body': { refChanges: [{refId: "refs/heads/bogus_branch", toHash: "0"}]},
+    'body': { refChanges: [{refId: "refs/heads/bogus_branch", toHash: "deadbeef"}, {refId: "refs/heads/boguser_branch", toHash: "deadbeef"}] },
     'fqurl': 'http://localhost:5253/stashpoke_bogus_branch',
@@ -131,3 +139,3 @@ 'no_change_expected': true
     'body': { ref: "refs/remotes/origin/master", head_commit: {id: 12345} },
-    'fqurl': 'http://localhost:5254/githubpoke_bogus_branch',
+    'fqurl': 'http://localhost:5254/githubpoke_bogus_ref',
     'no_change_expected': true
@@ -138,3 +146,3 @@ },{
     'port': 5254,
-    'body': { refChanges: [{refId: "refs/remotes/origin/master", toHash: "0"}]},
+    'body': { refChanges: [{refId: "refs/remotes/origin/master", toHash: "0"}] },
     'fqurl': 'http://localhost:5254/stashpoke_bogus_ref',
@@ -152,4 +160,2 @@ 'no_change_expected': true
 
-  var repo;
-
   describe('webhook', function() {
@@ -171,15 +177,19 @@
         if (config.type === 'stash') req_conf.headers = {'content-encoding':'UTF-8'};
+
         request(req_conf, function(err) {
 
-          if (err) return cb(err);
+          setTimeout(function() {
 
-          // If this is a test that won't trigger an update, such as a req specifying an untracked branch,
-          // short-circuit here and don't test for a KV update.
-          if (config.no_change_expected) return cb();
-
-          consul_utils.waitForValue('test_repo/master/' + sample_key, function(err) {
             if (err) return cb(err);
 
-            cb();
-          });
+            // If this is a test that won't trigger an update, such as a req specifying an untracked branch,
+            // short-circuit here and don't test for a KV update.
+            if (config.no_change_expected) return cb();
+
+            consul_utils.waitForValue('test_repo/master/' + sample_key, function(err) {
+              if (err) return cb(err);
+
+              cb();
+            });
+          }, 500);
         });
@@ -189,3 +199,3 @@ });
 
-    it ('should handle inbound requests', function(done) {
+    it ('should handle inbound requests with ' + test_descriptions[test_counter], function(done) {
       var repo_config = git_utils.createRepoConfig();
@@ -197,23 +207,19 @@ repo_config.hooks = hook_config;
 
-        // Handle the initial sync of this repo.  Init adds a file to the remote repo, and this line syncs
-        // that to our local cache and to consul.
-        repo.branches['master'].handleRefChange(0, function(err) {
-          if (err) return done(err);
+        // Test each hook config, 1 at a time.  Since they are updating the same repo, having all webhooks
+        // fire in parallel would lead to undefined results.
+        var test_config = function() {
+          config = hook_config.pop();
+          test_hook_req(config, function(err) {
+            if (err) return done(err);
 
-          // Test each hook config, 1 at a time.  Since they are updating the same repo, having all webhooks
-          // fire in parallel would lead to undefined results.
-          var test_config = function() {
-            config = hook_config.pop();
-            test_hook_req(config, function(err) {
-              if (err) return done(err);
+            if (hook_config.length > 0) return test_config();
+            done();
+          });
+        };
 
-              if (hook_config.length > 0) return test_config();
-              done();
-            });
-          };
-
-          test_config();
-        });
+        test_config();
       });
     });
+
+    ++test_counter;
   });
@@ -323,3 +329,2 @@ });
           done();
-
         });
@@ -326,0 +331,0 @@ });

No alert changes

Worsened metrics

Total package byte prevSize

102231

decreased by-0.97%

Lines of code

2267

decreased by-1%

No dependency changes