From 6b8b8b39b2c98b38948c5d5484cae3f2d90df25c Mon Sep 17 00:00:00 2001 From: Dean Sofer Date: Tue, 5 Dec 2017 17:11:02 -0800 Subject: [PATCH 1/7] [feature] Spy passes through calling with `new` This changes the spy behavior so that if the spy is `calledWithNew()` then the original function will also get called with `new`. This was throwing an error if you tried spying on an ES6 class object. --- lib/sinon/spy.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/sinon/spy.js b/lib/sinon/spy.js index 9a44a33be..841d4828a 100644 --- a/lib/sinon/spy.js +++ b/lib/sinon/spy.js @@ -190,9 +190,15 @@ var spyApi = { try { this.invoking = true; - returnValue = (this.func || func).apply(thisValue, args); - var thisCall = this.getCall(this.callCount - 1); + + if (thisCall.calledWithNew()) { + // Call through with `new` + returnValue = new (Function.prototype.bind.apply(this.func || func, [thisValue].concat(args))); + } else { + returnValue = (this.func || func).apply(thisValue, args); + } + if (thisCall.calledWithNew() && typeof returnValue !== "object") { returnValue = thisValue; } From e2efc656ecb65356c28506c26252697d3cbf6436 Mon Sep 17 00:00:00 2001 From: Dean Sofer Date: Wed, 6 Dec 2017 09:17:06 -0800 Subject: [PATCH 2/7] fixed PR 1626 build errors --- lib/sinon/spy.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/sinon/spy.js b/lib/sinon/spy.js index 841d4828a..4595a5d53 100644 --- a/lib/sinon/spy.js +++ b/lib/sinon/spy.js @@ -193,10 +193,11 @@ var spyApi = { var thisCall = this.getCall(this.callCount - 1); if (thisCall.calledWithNew()) { - // Call through with `new` - returnValue = new (Function.prototype.bind.apply(this.func || func, [thisValue].concat(args))); + // Call through with `new` + // eslint-disable-next-line new-parens + returnValue = new (Function.prototype.bind.apply(this.func || func, [thisValue].concat(args))); } else { - returnValue = (this.func || func).apply(thisValue, args); + returnValue = (this.func || func).apply(thisValue, args); } if (thisCall.calledWithNew() && typeof returnValue !== "object") { From 2f9b8f282e7056436c51722534ab4dfb4525c197 Mon Sep 17 00:00:00 2001 From: Dean Sofer Date: Wed, 6 Dec 2017 09:20:55 -0800 Subject: [PATCH 3/7] Merged thisCall.calledWithNew() if-statements --- lib/sinon/spy.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/sinon/spy.js b/lib/sinon/spy.js index 4595a5d53..07515213c 100644 --- a/lib/sinon/spy.js +++ b/lib/sinon/spy.js @@ -196,13 +196,13 @@ var spyApi = { // Call through with `new` // eslint-disable-next-line new-parens returnValue = new (Function.prototype.bind.apply(this.func || func, [thisValue].concat(args))); + + if (typeof returnValue !== "object") { + returnValue = thisValue; + } } else { returnValue = (this.func || func).apply(thisValue, args); } - - if (thisCall.calledWithNew() && typeof returnValue !== "object") { - returnValue = thisValue; - } } catch (e) { exception = e; } finally { From 3d26922a3560599b09bd95f9b8e813434bcae531 Mon Sep 17 00:00:00 2001 From: Dean Sofer Date: Wed, 6 Dec 2017 09:31:56 -0800 Subject: [PATCH 4/7] Added "spy passes 'new' to underlying function" test I hope this is a decent enough way to unit test this behavior --- test/spy-test.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/spy-test.js b/test/spy-test.js index 3d6e41a36..f253794f6 100644 --- a/test/spy-test.js +++ b/test/spy-test.js @@ -438,6 +438,17 @@ describe("spy", function () { assert(called); }); + + it("passes 'new' to underlying function", function () { + var called = false; + var TestClass = function(){}; + + var spy = createSpy.create(TestClass); + + var instance = new spy(); + + assert(instance instanceof TestClass); + }); it("passs arguments to function", function () { var actualArgs; From f01fb2936cb96fcd974e99c2f78d3cd8bd435d65 Mon Sep 17 00:00:00 2001 From: Dean Sofer Date: Wed, 6 Dec 2017 10:53:07 -0800 Subject: [PATCH 5/7] Removed trailing space in spy.js --- lib/sinon/spy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sinon/spy.js b/lib/sinon/spy.js index 07515213c..d2caf8a9e 100644 --- a/lib/sinon/spy.js +++ b/lib/sinon/spy.js @@ -196,7 +196,7 @@ var spyApi = { // Call through with `new` // eslint-disable-next-line new-parens returnValue = new (Function.prototype.bind.apply(this.func || func, [thisValue].concat(args))); - + if (typeof returnValue !== "object") { returnValue = thisValue; } From 3526a6b63986c50a34a8d160cd7b21e6de9129b1 Mon Sep 17 00:00:00 2001 From: Dean Sofer Date: Wed, 6 Dec 2017 10:55:31 -0800 Subject: [PATCH 6/7] Fixed lint errors in spy-test.js --- test/spy-test.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/test/spy-test.js b/test/spy-test.js index f253794f6..81bef83b1 100644 --- a/test/spy-test.js +++ b/test/spy-test.js @@ -438,14 +438,13 @@ describe("spy", function () { assert(called); }); - + it("passes 'new' to underlying function", function () { - var called = false; - var TestClass = function(){}; + var TestClass = function () {}; + + var SpyClass = createSpy.create(TestClass); - var spy = createSpy.create(TestClass); - - var instance = new spy(); + var instance = new SpyClass(); assert(instance instanceof TestClass); }); From 478e58a1c4d9b3596f15c98f549b99545d42d100 Mon Sep 17 00:00:00 2001 From: Carl-Erik Kopseng Date: Thu, 7 Dec 2017 14:05:53 +0100 Subject: [PATCH 7/7] Extract method from prototype and make test easier to debug Using a named function makes it far easier to follow what is happening in a debugger. --- lib/sinon/spy.js | 3 ++- test/spy-test.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/sinon/spy.js b/lib/sinon/spy.js index d2caf8a9e..29c612170 100644 --- a/lib/sinon/spy.js +++ b/lib/sinon/spy.js @@ -17,6 +17,7 @@ var push = Array.prototype.push; var slice = Array.prototype.slice; var filter = Array.prototype.filter; var ErrorConstructor = Error.prototype.constructor; +var bind = Function.prototype.bind; var callId = 0; @@ -195,7 +196,7 @@ var spyApi = { if (thisCall.calledWithNew()) { // Call through with `new` // eslint-disable-next-line new-parens - returnValue = new (Function.prototype.bind.apply(this.func || func, [thisValue].concat(args))); + returnValue = new (bind.apply(this.func || func, [thisValue].concat(args)))(); if (typeof returnValue !== "object") { returnValue = thisValue; diff --git a/test/spy-test.js b/test/spy-test.js index 81bef83b1..dc68d6d7f 100644 --- a/test/spy-test.js +++ b/test/spy-test.js @@ -440,7 +440,7 @@ describe("spy", function () { }); it("passes 'new' to underlying function", function () { - var TestClass = function () {}; + function TestClass() {} var SpyClass = createSpy.create(TestClass);