Skip to content
Browse files

Fixing bug with false positives in cyclic dependency test (fixes issue

  • Loading branch information...
1 parent 8d18384 commit 61adc6d42ec08d9619eab438622ee8d443ec2380 @TrevorBurnham committed Nov 2, 2011
Showing with 48 additions and 17 deletions.
  1. +12 −6 docs/dep-graph.html
  2. +9 −5 lib/dep-graph.js
  3. +12 −6 src/dep-graph.coffee
  4. +15 −0 test/test.coffee
View
18 docs/dep-graph.html
@@ -6,17 +6,23 @@
<span class="nx">@map</span><span class="p">[</span><span class="nx">id</span><span class="p">].</span><span class="nx">push</span> <span class="nx">depId</span>
<span class="nx">@map</span><span class="p">[</span><span class="nx">id</span><span class="p">]</span></pre></div> </td> </tr> <tr id="section-4"> <td class="docs"> <div class="pilwrap"> <a class="pilcrow" href="#section-4">&#182;</a> </div> <p>Generate a list of all dependencies (direct and indirect) for the given id,
in logical order with no duplicates.</p> </td> <td class="code"> <div class="highlight"><pre> <span class="nv">getChain: </span><span class="nf">(id, traversedPaths = [], traversedBranch = []) -&gt;</span>
+ <span class="nx">traversedPaths</span><span class="p">.</span><span class="nx">unshift</span> <span class="nx">id</span>
+ <span class="nx">traversedBranch</span><span class="p">.</span><span class="nx">unshift</span> <span class="nx">id</span>
<span class="k">return</span> <span class="p">[]</span> <span class="nx">unless</span> <span class="nx">@map</span><span class="p">[</span><span class="nx">id</span><span class="p">]</span>
- <span class="k">for</span> <span class="nx">depId</span> <span class="k">in</span> <span class="nx">@map</span><span class="p">[</span><span class="nx">id</span><span class="p">].</span><span class="nx">slice</span><span class="p">(</span><span class="mi">0</span><span class="p">).</span><span class="nx">reverse</span><span class="p">()</span>
+ <span class="nv">depIds = </span><span class="nx">@map</span><span class="p">[</span><span class="nx">id</span><span class="p">]</span>
+ <span class="k">for</span> <span class="nx">depId</span> <span class="k">in</span> <span class="nx">depIds</span><span class="p">.</span><span class="nx">slice</span><span class="p">(</span><span class="mi">0</span><span class="p">).</span><span class="nx">reverse</span><span class="p">()</span>
<span class="k">if</span> <span class="nx">depId</span> <span class="k">in</span> <span class="nx">traversedBranch</span> <span class="c1"># cycle</span>
- <span class="k">throw</span> <span class="k">new</span> <span class="nb">Error</span><span class="p">(</span><span class="s2">&quot;Cyclic dependency from #{id} to #{depId}&quot;</span><span class="p">)</span>
- <span class="k">continue</span> <span class="k">if</span> <span class="nx">depId</span> <span class="k">in</span> <span class="nx">traversedPaths</span> <span class="c1"># duplicate</span>
- <span class="nx">traversedPaths</span><span class="p">.</span><span class="nx">unshift</span> <span class="nx">depId</span>
- <span class="nx">traversedBranch</span><span class="p">.</span><span class="nx">unshift</span> <span class="nx">depId</span>
+ <span class="k">throw</span> <span class="k">new</span> <span class="nb">Error</span><span class="p">(</span><span class="s2">&quot;Cyclic dependency from #{id} to #{depId}&quot;</span><span class="p">)</span>
+ <span class="k">if</span> <span class="nx">depId</span> <span class="k">in</span> <span class="nx">traversedPaths</span> <span class="c1"># duplicate, push to front</span>
+ <span class="nv">depIdIndex = </span><span class="nx">traversedPaths</span><span class="p">.</span><span class="nx">indexOf</span> <span class="nx">depId</span>
+ <span class="nx">traversedPaths</span><span class="p">[</span><span class="nx">depIdIndex</span><span class="p">..</span><span class="nx">depIdIndex</span><span class="p">]</span> <span class="o">=</span> <span class="p">[]</span>
+ <span class="nx">traversedPaths</span><span class="p">.</span><span class="nx">unshift</span> <span class="nx">depId</span>
+ <span class="k">continue</span>
+
<span class="nx">@getChain</span> <span class="nx">depId</span><span class="p">,</span> <span class="nx">traversedPaths</span><span class="p">,</span> <span class="nx">traversedBranch</span><span class="p">.</span><span class="nx">slice</span><span class="p">(</span><span class="mi">0</span><span class="p">)</span>
- <span class="nx">traversedPaths</span></pre></div> </td> </tr> <tr id="section-5"> <td class="docs"> <div class="pilwrap"> <a class="pilcrow" href="#section-5">&#182;</a> </div> <p>Export the class in Node, make it global in the browser.</p> </td> <td class="code"> <div class="highlight"><pre><span class="k">if</span> <span class="nx">module</span><span class="o">?</span><span class="p">.</span><span class="nx">exports</span><span class="o">?</span>
+ <span class="nx">traversedPaths</span><span class="p">[</span><span class="mi">0</span><span class="p">...</span><span class="o">-</span><span class="mi">1</span><span class="p">]</span></pre></div> </td> </tr> <tr id="section-5"> <td class="docs"> <div class="pilwrap"> <a class="pilcrow" href="#section-5">&#182;</a> </div> <p>Export the class in Node, make it global in the browser.</p> </td> <td class="code"> <div class="highlight"><pre><span class="k">if</span> <span class="nx">module</span><span class="o">?</span><span class="p">.</span><span class="nx">exports</span><span class="o">?</span>
<span class="nv">module.exports = </span><span class="nx">DepGraph</span>
<span class="k">else</span>
<span class="vi">@DepGraph = </span><span class="nx">DepGraph</span>
View
14 lib/dep-graph.js
@@ -22,30 +22,34 @@
return this.map[id];
};
DepGraph.prototype.getChain = function(id, traversedPaths, traversedBranch) {
- var depId, _i, _len, _ref;
+ var depId, depIdIndex, depIds, _i, _len, _ref, _ref2;
if (traversedPaths == null) {
traversedPaths = [];
}
if (traversedBranch == null) {
traversedBranch = [];
}
+ traversedPaths.unshift(id);
+ traversedBranch.unshift(id);
if (!this.map[id]) {
return [];
}
- _ref = this.map[id].slice(0).reverse();
+ depIds = this.map[id];
+ _ref = depIds.slice(0).reverse();
for (_i = 0, _len = _ref.length; _i < _len; _i++) {
depId = _ref[_i];
if (__indexOf.call(traversedBranch, depId) >= 0) {
throw new Error("Cyclic dependency from " + id + " to " + depId);
}
if (__indexOf.call(traversedPaths, depId) >= 0) {
+ depIdIndex = traversedPaths.indexOf(depId);
+ [].splice.apply(traversedPaths, [depIdIndex, depIdIndex - depIdIndex + 1].concat(_ref2 = [])), _ref2;
+ traversedPaths.unshift(depId);
continue;
}
- traversedPaths.unshift(depId);
- traversedBranch.unshift(depId);
this.getChain(depId, traversedPaths, traversedBranch.slice(0));
}
- return traversedPaths;
+ return traversedPaths.slice(0, -1);
};
return DepGraph;
})();
View
18 src/dep-graph.coffee
@@ -16,17 +16,23 @@ class DepGraph
# Generate a list of all dependencies (direct and indirect) for the given id,
# in logical order with no duplicates.
getChain: (id, traversedPaths = [], traversedBranch = []) ->
+ traversedPaths.unshift id
+ traversedBranch.unshift id
return [] unless @map[id]
- for depId in @map[id].slice(0).reverse()
+ depIds = @map[id]
+ for depId in depIds.slice(0).reverse()
if depId in traversedBranch # cycle
- throw new Error("Cyclic dependency from #{id} to #{depId}")
- continue if depId in traversedPaths # duplicate
- traversedPaths.unshift depId
- traversedBranch.unshift depId
+ throw new Error("Cyclic dependency from #{id} to #{depId}")
+ if depId in traversedPaths # duplicate, push to front
+ depIdIndex = traversedPaths.indexOf depId
+ traversedPaths[depIdIndex..depIdIndex] = []
+ traversedPaths.unshift depId
+ continue
+
@getChain depId, traversedPaths, traversedBranch.slice(0)
- traversedPaths
+ traversedPaths[0...-1]
# Export the class in Node, make it global in the browser.
if module?.exports?
View
15 test/test.coffee
@@ -17,4 +17,19 @@ exports['Indirect dependencies are chained before their dependents'] = (test) ->
exports['getChain can safely be called for unknown resources'] = (test) ->
test.doesNotThrow -> depGraph.getChain('Z')
test.deepEqual depGraph.getChain('Z'), []
+ test.done()
+
+exports['Cyclic dependencies are detected'] = (test) ->
+ depGraph.add 'yin', 'yang'
+ depGraph.add 'yang', 'yin'
+ test.throws -> depGraph.getChain 'yin'
+ test.throws -> depGraph.getChain 'yang'
+ test.done()
+
+exports['Arc direction is taken into account (issue #1)'] = (test) ->
+ depGraph.add "MAIN", "One"
+ depGraph.add "MAIN", "Three"
+ depGraph.add "One", "Two"
+ depGraph.add "Two", "Three"
+ test.deepEqual depGraph.getChain("MAIN"), ['Three', 'Two', 'One']
test.done()

0 comments on commit 61adc6d

Please sign in to comment.
Something went wrong with that request. Please try again.