Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sandbox not restoring stubbed constructors in 1.17 #851

Closed
neckardt opened this issue Sep 23, 2015 · 3 comments
Closed

Sandbox not restoring stubbed constructors in 1.17 #851

neckardt opened this issue Sep 23, 2015 · 3 comments

Comments

@neckardt
Copy link

Our tests suddenly started failing after upgrading to the newest version of sinon. Stubbing objects that have a constructor while using the sandbox does not work properly -- sandbox.restore() does not restore the constructor. This issue does not occur in 1.16.

An example which replicates this issue:

describe.only('sandbox-test', function(){
  function MyClass(){}
  it('should not throw an error', function(){
    var myObject = new MyClass();
    var sandbox = sinon.sandbox.create();
    sandbox.stub(myObject);
    sandbox.restore();
    sandbox.stub(myObject);
  });
});

The error that occurs:

 sandbox-test
    1) should not throw an error


  0 passing (86ms)
  1 failing

  1) sandbox-test should not throw an error:
     TypeError: Attempted to wrap constructor which is already wrapped
      at checkWrappedMethod (node_modules/sinon/lib/sinon/util/core.js:81:29)
      at Object.wrapMethod (node_modules/sinon/lib/sinon/util/core.js:121:21)
      at stub (node_modules/sinon/lib/sinon/stub.js:66:26)
      at node_modules/sinon/lib/sinon/stub.js:59:25
      at node_modules/sinon/lib/sinon/walk.js:34:26
      at Array.forEach (native)
      at Object.walk (node_modules/sinon/lib/sinon/walk.js:33:45)
      at Object.stub (node_modules/sinon/lib/sinon/stub.js:52:23)
      at Object.stub (node_modules/sinon/lib/sinon/collection.js:106:49)
      at Context.<anonymous> (test/unit/sandbox-test.js:16:13)
@mroderick
Copy link
Member

This looks like it's related to #847, #852

@traviskaufman
Copy link
Contributor

Submitted PR which includes a fix. Also added a test in there that tests #852. Not sure this will fix #847 but I'm tackling that one next.

@mroderick
Copy link
Member

Sinon.JS v1.17.1 is now on npm, the website will be updated soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants