busterjs / buster

Abandoned - A powerful suite of automated test tools for JavaScript.
http://docs.busterjs.org
Other
448 stars 37 forks source link

Cyclical data structure with assert.calledWith yields "InternalError: too much recursion" #124

Closed ende76 closed 12 years ago

ende76 commented 12 years ago

Description

When implementing cyclical data structures (doubly-linked lists, tree structures with parent references), and then using assert.calledWith() on a spy/stub, an InternalError is thrown.

assert.equals() appears to have the same problem.

Example

A minimal example can be found at https://github.com/ende76/buster.internalerror . The test treenode module-> method has() -> should call find() on child nodes should fail, but instead errors.

Naive analysis

From an outside view, it seems that in deep-comparison cases, cyclical structures would have to be recognized and avoided, which might be non-trivial.

meisl commented 12 years ago

node's debugger gets it right, maybe it's possible to get hands on that? ...but really only "maybe" since this might well be internal to V8.

Here's the quick example I tried (on Win, node 0.6.11):

//vicious.js
var heaven = {
  temperature: 'convenient'
};
var hell = {
  temperature: 'well, not so...',
  opposite: heaven
};
heaven.opposite = hell;
debugger;

yields when run:

d:\>node debug vicious
< debugger listening on port 5858
connecting... ok
debug> c
break in d:\vicious.js:10
  8 };
  9 heaven.opposite = hell;
 10 debugger;
 11
 12 });
debug> repl
Press Ctrl + C to leave debug repl
> hell
{ temperature: 'well, not so...',
  opposite:
   { temperature: 'convenient',
     opposite:
      { temperature: 'well, not so...',
        opposite: [Object] } } }
> heaven
{ temperature: 'convenient',
  opposite:
   { temperature: 'well, not so...',
     opposite:
      { temperature: 'convenient',
        opposite: [Object] } } }
>
cjohansen commented 12 years ago

I just did a quick test locally with @meisl's example (heaven and hell), and cannot reproduce the error with neither assert.equals nor calledWith, but I'll investigate @ende76's example project too.

cjohansen commented 12 years ago

@ende76 Your sample project fails on my machine (as expected). Can you confirm that this bug is fixed?

ende76 commented 12 years ago

@cjohansen Sorry, there was a small problem with a non-necessary and non-existent dependency in buster.js, which might have looked like a correct test failure. I have fixed the configuration now, and still get the internal error.

Firefox 13.0.1, OS X 10.7 (Lion): .......E...                                                                      
Error: Firefox 13.0.1, OS X 10.7 (Lion) treenode module method has() should call find() on child nodes
InternalError: too much recursion

1 test case, 11 tests, 11 assertions, 0 failures, 1 error, 0 timeouts
Finished in 0.494s`
ende76 commented 12 years ago

@cjohansen To make sure that you're not hunting bugs that only exist because I'm on an obsolete version, here's some relevant output:

Lily:buster.internalerror thpickert$ buster -v
Buster.JS version 0.6.0 Beta 4
Lily:buster.internalerror thpickert$ node -v
v0.8.1
cjohansen commented 12 years ago

Figured it out. It has already been solved. You can verify by doing this:

git clone git://github.com/ende76/buster.internalerror.git
cd buster.internalerror/node_modules/buster/node_modules
rm -fr buster-format
git clone https://github.com/busterjs/buster-format.git
cd ../../../
./node_modules/buster/bin/buster-test