feat: add hasJoin function and fix hasCycle

index.js

@@ -3,5 +3,6 @@
 const getWorkflow = require('./lib/getWorkflow');
 const getNextJobs = require('./lib/getNextJobs');
 const hasCycle = require('./lib/hasCycle');
+const hasJoin = require('./lib/hasJoin');
 
-module.exports = { getWorkflow, getNextJobs, hasCycle };
+module.exports = { getWorkflow, getNextJobs, hasCycle, hasJoin };

lib/hasCycle.js

@@ -8,26 +8,31 @@ const getNextJobs = require('./getNextJobs');
  * @param  {Object} workflowGraph   Directed graph representation of workflow
  * @param  {String} jobName         Job Name
  * @param  {Set} visitedJobs        Unique list of visited jobs
+ * @param  {Array} path             Recursive stack of a single path from node to leaf
  * @return {Boolean}                True if a cycle detected
  * @private
  */
-const walk = (workflowGraph, jobName, visitedJobs) => {
+const isCyclic = (workflowGraph, jobName, visitedJobs, path) => {
     visitedJobs.add(jobName);
+    path.push(jobName);
 
     const triggerList = getNextJobs(workflowGraph, { trigger: jobName, prNum: 1 });
 
-    // Hit a leaf node, must be good
-    if (triggerList.length === 0) {
+    const hasCycle = triggerList.some((name) => {
+        if (!visitedJobs.has(name)) {
+            return isCyclic(workflowGraph, name, visitedJobs, path);
+        } else if (path.includes(name)) {
+            // When a job is visited and is in the current path, then cycle detected
+            return true;
+        }
+
         return false;
-    }
+    });
 
-    // Check to see if we visited this job before
-    if (triggerList.some(name => visitedJobs.has(name))) {
-        return true;
-    }
+    // Remove job from current path
+    path.pop(jobName);
 
-    // recursively walk starting from the new jobs
-    return triggerList.some(name => walk(workflowGraph, name, visitedJobs));
+    return hasCycle;
 };
 
 /**
@@ -38,6 +43,6 @@ const walk = (workflowGraph, jobName, visitedJobs) => {
  */
 const hasCycle = workflowGraph =>
     // Check from all the nodes to capture deteached workflows
-    workflowGraph.nodes.some(node => walk(workflowGraph, node.name, new Set()));
+    workflowGraph.nodes.some(node => isCyclic(workflowGraph, node.name, new Set(), []));
 
 module.exports = hasCycle;

lib/hasJoin.js

@@ -0,0 +1,13 @@
+'use strict';
+
+/**
+ * Calculate if the workflow contains a cycle, e.g. A -> B -> A
+ * @method hasCycle
+ * @param  {Object}  workflowGraph  Graph representation of workflow
+ * @return {Boolean}                True if a cycle exists anywhere in the workflow
+ */
+const hasJoin = workflowGraph =>
+    // Check from all the nodes to capture deteached workflows
+    workflowGraph.edges.some(edge => edge.join);
+
+module.exports = hasJoin;

test/lib/hasCycle.test.js

@@ -84,4 +84,24 @@ describe('hasCyles', () => {
 
         assert.isFalse(hasCycle(workflow));
     });
+
+    it('should return false if workflow contains join but no cycle', () => {
+        const workflow = {
+            nodes: [
+                { name: '~pr' },
+                { name: '~commit' },
+                { name: 'A' },
+                { name: 'B' },
+                { name: 'C' }
+            ],
+            edges: [
+                { src: '~commit', dest: 'A' }, // start
+                { src: '~commit', dest: 'B' }, // start
+                { src: 'A', dest: 'C', join: true }, // join
+                { src: 'B', dest: 'C', join: true } // join
+            ]
+        };
+
+        assert.isFalse(hasCycle(workflow));
+    });
 });

test/lib/hasJoin.test.js

@@ -0,0 +1,42 @@
+'use strict';
+
+const assert = require('chai').assert;
+const hasJoin = require('../../lib/hasJoin');
+
+describe('hasJoin', () => {
+    it('should return true if a workflow has a join', () => {
+        const workflow = {
+            nodes: [
+                { name: '~pr' },
+                { name: '~commit' },
+                { name: 'A' },
+                { name: 'B' },
+                { name: 'C' }
+            ],
+            edges: [
+                { src: '~commit', dest: 'A' }, // start
+                { src: 'A', dest: 'C', join: true }, // join
+                { src: 'B', dest: 'C', join: true } // join
+            ]
+        };
+
+        assert.isTrue(hasJoin(workflow));
+    });
+
+    it('should return false if workflow has no join', () => {
+        const workflow = {
+            nodes: [
+                { name: '~pr' },
+                { name: '~commit' },
+                { name: 'A' },
+                { name: 'B' }
+            ],
+            edges: [
+                { src: '~commit', dest: 'A' }, // start
+                { src: 'A', dest: 'B' } // end
+            ]
+        };
+
+        assert.isFalse(hasJoin(workflow));
+    });
+});