From 9febe3159d729bdf9459274216ffe4cbf3588dd7 Mon Sep 17 00:00:00 2001 From: Elliot Nelson Date: Tue, 28 Jan 2020 19:40:44 -0500 Subject: [PATCH 1/3] Allow callThrough to call constructor functions without errors --- spec/core/integration/EnvSpec.js | 19 +++++++++++ src/core/Spy.js | 56 ++++++++++++++++---------------- src/core/SpyStrategy.js | 11 +++++-- 3 files changed, 56 insertions(+), 30 deletions(-) diff --git a/spec/core/integration/EnvSpec.js b/spec/core/integration/EnvSpec.js index 490e9d94..58f06014 100644 --- a/spec/core/integration/EnvSpec.js +++ b/spec/core/integration/EnvSpec.js @@ -808,6 +808,25 @@ describe("Env integration", function() { expect(originalFunctionWasCalled).toEqual(true); }); + env.it("works with constructors when using callThrough spy strategy", function() { + function MyClass(foo) { + if (!(this instanceof MyClass)) throw new Error('You must use the new keyword.'); + this.foo = foo; + } + var subject = { MyClass: MyClass }; + var spy = env.spyOn(subject, 'MyClass').and.callThrough(); + + expect(function() { + var result = new spy('hello world'); + expect(result instanceof MyClass).toBeTruthy(); + expect(result.foo).toEqual('hello world'); + }).not.toThrow(); + + expect(function() { + spy('hello world'); + }).toThrowError('You must use the new keyword.'); + }); + env.execute(); }); diff --git a/src/core/Spy.js b/src/core/Spy.js index ffbb0103..01c772fe 100644 --- a/src/core/Spy.js +++ b/src/core/Spy.js @@ -20,8 +20,8 @@ getJasmineRequireObj().Spy = function(j$) { getPromise ) { var numArgs = typeof originalFn === 'function' ? originalFn.length : 0, - wrapper = makeFunc(numArgs, function() { - return spy.apply(this, Array.prototype.slice.call(arguments)); + wrapper = makeFunc(numArgs, function(context, args, isConstructor) { + return spy(context, args, isConstructor); }), strategyDispatcher = new SpyStrategyDispatcher({ name: name, @@ -33,7 +33,7 @@ getJasmineRequireObj().Spy = function(j$) { getPromise: getPromise }), callTracker = new j$.CallTracker(), - spy = function() { + spy = function(context, args, isConstructor) { /** * @name Spy.callData * @property {object} object - `this` context for the invocation. @@ -41,13 +41,13 @@ getJasmineRequireObj().Spy = function(j$) { * @property {Array} args - The arguments passed for this invocation. */ var callData = { - object: this, + object: context, invocationOrder: nextOrder(), - args: Array.prototype.slice.apply(arguments) + args: Array.prototype.slice.apply(args) }; callTracker.track(callData); - var returnValue = strategyDispatcher.exec(this, arguments); + var returnValue = strategyDispatcher.exec(this, args, isConstructor); callData.returnValue = returnValue; return returnValue; @@ -56,44 +56,44 @@ getJasmineRequireObj().Spy = function(j$) { function makeFunc(length, fn) { switch (length) { case 1: - return function(a) { - return fn.apply(this, arguments); + return function fnargs(a) { + return fn(this, arguments, this instanceof fnargs); }; case 2: - return function(a, b) { - return fn.apply(this, arguments); + return function fnargs(a, b) { + return fn(this, arguments, this instanceof fnargs); }; case 3: - return function(a, b, c) { - return fn.apply(this, arguments); + return function fnargs(a, b, c) { + return fn(this, arguments, this instanceof fnargs); }; case 4: - return function(a, b, c, d) { - return fn.apply(this, arguments); + return function fnargs(a, b, c, d) { + return fn(this, arguments, this instanceof fnargs); }; case 5: - return function(a, b, c, d, e) { - return fn.apply(this, arguments); + return function fnargs(a, b, c, d, e) { + return fn(this, arguments, this instanceof fnargs); }; case 6: - return function(a, b, c, d, e, f) { - return fn.apply(this, arguments); + return function fnargs(a, b, c, d, e, f) { + return fn(this, arguments, this instanceof fnargs); }; case 7: - return function(a, b, c, d, e, f, g) { - return fn.apply(this, arguments); + return function fnargs(a, b, c, d, e, f, g) { + return fn(this, arguments, this instanceof fnargs); }; case 8: - return function(a, b, c, d, e, f, g, h) { - return fn.apply(this, arguments); + return function fnargs(a, b, c, d, e, f, g, h) { + return fn(this, arguments, this instanceof fnargs); }; case 9: - return function(a, b, c, d, e, f, g, h, i) { - return fn.apply(this, arguments); + return function fnargs(a, b, c, d, e, f, g, h, i) { + return fn(this, arguments, this instanceof fnargs); }; default: - return function() { - return fn.apply(this, arguments); + return function fnargs() { + return fn(this, arguments, this instanceof fnargs); }; } } @@ -150,7 +150,7 @@ getJasmineRequireObj().Spy = function(j$) { this.and = baseStrategy; - this.exec = function(spy, args) { + this.exec = function(spy, args, isConstructor) { var strategy = argsStrategies.get(args); if (!strategy) { @@ -167,7 +167,7 @@ getJasmineRequireObj().Spy = function(j$) { } } - return strategy.exec(spy, args); + return strategy.exec(spy, args, isConstructor); }; this.withArgs = function() { diff --git a/src/core/SpyStrategy.js b/src/core/SpyStrategy.js index 83985967..252de41e 100644 --- a/src/core/SpyStrategy.js +++ b/src/core/SpyStrategy.js @@ -96,8 +96,15 @@ getJasmineRequireObj().SpyStrategy = function(j$) { * @since 2.0.0 * @function */ - SpyStrategy.prototype.exec = function(context, args) { - return this.plan.apply(context, args); + SpyStrategy.prototype.exec = function(context, args, isConstructor) { + var list = [context].concat(args ? Array.prototype.slice.call(args) : []); + var target = this.plan.bind.apply(this.plan, list); + + if (isConstructor) { + return new target(); + } else { + return target(); + } }; /** From bf4694333cde39049818e7f4768a47d345275a94 Mon Sep 17 00:00:00 2001 From: Elliot Nelson Date: Thu, 6 Feb 2020 10:29:02 -0500 Subject: [PATCH 2/3] Improve wrapper function and parameter naming --- spec/core/integration/EnvSpec.js | 10 ++++-- src/core/Spy.js | 52 ++++++++++++++++---------------- src/core/SpyStrategy.js | 4 +-- 3 files changed, 36 insertions(+), 30 deletions(-) diff --git a/spec/core/integration/EnvSpec.js b/spec/core/integration/EnvSpec.js index 58f06014..d3d347fc 100644 --- a/spec/core/integration/EnvSpec.js +++ b/spec/core/integration/EnvSpec.js @@ -822,6 +822,12 @@ describe("Env integration", function() { expect(result.foo).toEqual('hello world'); }).not.toThrow(); + expect(function() { + var result = new spy('passing', 'extra', 'arguments', 'to', 'constructor'); + expect(result instanceof MyClass).toBeTruthy(); + expect(result.foo).toEqual('passing'); + }).not.toThrow(); + expect(function() { spy('hello world'); }).toThrowError('You must use the new keyword.'); @@ -840,8 +846,8 @@ describe("Env integration", function() { env.allowRespy(true); env.addReporter({ jasmineDone: done }); - env.describe('test suite', function(){ - env.it('spec 0', function(){ + env.describe('test suite', function() { + env.it('spec 0', function() { env.spyOn(foo,'bar'); var error = null; diff --git a/src/core/Spy.js b/src/core/Spy.js index 01c772fe..377b3f57 100644 --- a/src/core/Spy.js +++ b/src/core/Spy.js @@ -20,8 +20,8 @@ getJasmineRequireObj().Spy = function(j$) { getPromise ) { var numArgs = typeof originalFn === 'function' ? originalFn.length : 0, - wrapper = makeFunc(numArgs, function(context, args, isConstructor) { - return spy(context, args, isConstructor); + wrapper = makeFunc(numArgs, function(context, args, invokeNew) { + return spy(context, args, invokeNew); }), strategyDispatcher = new SpyStrategyDispatcher({ name: name, @@ -33,7 +33,7 @@ getJasmineRequireObj().Spy = function(j$) { getPromise: getPromise }), callTracker = new j$.CallTracker(), - spy = function(context, args, isConstructor) { + spy = function(context, args, invokeNew) { /** * @name Spy.callData * @property {object} object - `this` context for the invocation. @@ -47,7 +47,7 @@ getJasmineRequireObj().Spy = function(j$) { }; callTracker.track(callData); - var returnValue = strategyDispatcher.exec(this, args, isConstructor); + var returnValue = strategyDispatcher.exec(context, args, invokeNew); callData.returnValue = returnValue; return returnValue; @@ -56,44 +56,44 @@ getJasmineRequireObj().Spy = function(j$) { function makeFunc(length, fn) { switch (length) { case 1: - return function fnargs(a) { - return fn(this, arguments, this instanceof fnargs); + return function wrap1(a) { + return fn(this, arguments, this instanceof wrap1); }; case 2: - return function fnargs(a, b) { - return fn(this, arguments, this instanceof fnargs); + return function wrap2(a, b) { + return fn(this, arguments, this instanceof wrap2); }; case 3: - return function fnargs(a, b, c) { - return fn(this, arguments, this instanceof fnargs); + return function wrap3(a, b, c) { + return fn(this, arguments, this instanceof wrap3); }; case 4: - return function fnargs(a, b, c, d) { - return fn(this, arguments, this instanceof fnargs); + return function wrap4(a, b, c, d) { + return fn(this, arguments, this instanceof wrap4); }; case 5: - return function fnargs(a, b, c, d, e) { - return fn(this, arguments, this instanceof fnargs); + return function wrap5(a, b, c, d, e) { + return fn(this, arguments, this instanceof wrap5); }; case 6: - return function fnargs(a, b, c, d, e, f) { - return fn(this, arguments, this instanceof fnargs); + return function wrap6(a, b, c, d, e, f) { + return fn(this, arguments, this instanceof wrap6); }; case 7: - return function fnargs(a, b, c, d, e, f, g) { - return fn(this, arguments, this instanceof fnargs); + return function wrap7(a, b, c, d, e, f, g) { + return fn(this, arguments, this instanceof wrap7); }; case 8: - return function fnargs(a, b, c, d, e, f, g, h) { - return fn(this, arguments, this instanceof fnargs); + return function wrap8(a, b, c, d, e, f, g, h) { + return fn(this, arguments, this instanceof wrap8); }; case 9: - return function fnargs(a, b, c, d, e, f, g, h, i) { - return fn(this, arguments, this instanceof fnargs); + return function wrap9(a, b, c, d, e, f, g, h, i) { + return fn(this, arguments, this instanceof wrap9); }; default: - return function fnargs() { - return fn(this, arguments, this instanceof fnargs); + return function wrap() { + return fn(this, arguments, this instanceof wrap); }; } } @@ -150,7 +150,7 @@ getJasmineRequireObj().Spy = function(j$) { this.and = baseStrategy; - this.exec = function(spy, args, isConstructor) { + this.exec = function(spy, args, invokeNew) { var strategy = argsStrategies.get(args); if (!strategy) { @@ -167,7 +167,7 @@ getJasmineRequireObj().Spy = function(j$) { } } - return strategy.exec(spy, args, isConstructor); + return strategy.exec(spy, args, invokeNew); }; this.withArgs = function() { diff --git a/src/core/SpyStrategy.js b/src/core/SpyStrategy.js index 252de41e..43f89831 100644 --- a/src/core/SpyStrategy.js +++ b/src/core/SpyStrategy.js @@ -96,11 +96,11 @@ getJasmineRequireObj().SpyStrategy = function(j$) { * @since 2.0.0 * @function */ - SpyStrategy.prototype.exec = function(context, args, isConstructor) { + SpyStrategy.prototype.exec = function(context, args, invokeNew) { var list = [context].concat(args ? Array.prototype.slice.call(args) : []); var target = this.plan.bind.apply(this.plan, list); - if (isConstructor) { + if (invokeNew) { return new target(); } else { return target(); From be3c8275d480e843f49cb64179022b2e0a86059a Mon Sep 17 00:00:00 2001 From: Elliot Nelson Date: Thu, 6 Feb 2020 10:34:55 -0500 Subject: [PATCH 3/3] Tidy up SpyStrategy#exec --- src/core/SpyStrategy.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/core/SpyStrategy.js b/src/core/SpyStrategy.js index 43f89831..7e86abd6 100644 --- a/src/core/SpyStrategy.js +++ b/src/core/SpyStrategy.js @@ -97,14 +97,12 @@ getJasmineRequireObj().SpyStrategy = function(j$) { * @function */ SpyStrategy.prototype.exec = function(context, args, invokeNew) { - var list = [context].concat(args ? Array.prototype.slice.call(args) : []); - var target = this.plan.bind.apply(this.plan, list); + var contextArgs = [context].concat( + args ? Array.prototype.slice.call(args) : [] + ); + var target = this.plan.bind.apply(this.plan, contextArgs); - if (invokeNew) { - return new target(); - } else { - return target(); - } + return invokeNew ? new target() : target(); }; /**