From 62ee2e001d9174f171d10fc873b481931de2e889 Mon Sep 17 00:00:00 2001 From: takasmiley Date: Mon, 27 Feb 2017 16:32:50 +0900 Subject: [PATCH 1/8] Add test for issue #1274 --- test/issues/issues-test.js | 45 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test/issues/issues-test.js b/test/issues/issues-test.js index 99461e547..7957f3c36 100644 --- a/test/issues/issues-test.js +++ b/test/issues/issues-test.js @@ -270,4 +270,49 @@ describe("issues", function () { }); }); } + + describe("#1274 - spy.withArgs(args...).callCount is incorrect", function () { + it("case1: should count accurately", function () { + var obj = { + f: function () {} + }; + + // case1: withArgs(1) => withArgs(1, 1) + var spy = sinon.spy(obj, "f"); + assert.equals(spy.callCount, 0); + assert.equals(spy.withArgs(1).callCount, 0); + assert.equals(spy.withArgs(1, 1).callCount, 0); + + obj.f(); + obj.f(1); + obj.f(1, 1); + obj.f(1, 2); + + assert.equals(spy.callCount, 4); + assert.equals(spy.withArgs(1).callCount, 3); + assert.equals(spy.withArgs(1, 1).callCount, 1); + assert.equals(spy.withArgs(1, 2).callCount, 1); + }); + it("case2: should count accurately", function () { + var obj = { + f: function () {} + }; + + // case2: withArgs(1, 1) => withArgs(1) + var spy = sinon.spy(obj, "f"); + assert.equals(spy.callCount, 0); + assert.equals(spy.withArgs(1, 1).callCount, 0); + assert.equals(spy.withArgs(1).callCount, 0); + + obj.f(); + obj.f(1); + obj.f(1, 1); + obj.f(1, 2); + + assert.equals(spy.callCount, 4); + assert.equals(spy.withArgs(1).callCount, 3); + assert.equals(spy.withArgs(1, 1).callCount, 1); + assert.equals(spy.withArgs(1, 2).callCount, 1); + }); + }); }); From 0e77b31bdc7381ead058bc6dc4c7657f0f1be08c Mon Sep 17 00:00:00 2001 From: takasmiley Date: Tue, 28 Feb 2017 12:23:55 +0900 Subject: [PATCH 2/8] Fix #1274: spy.withArgs(args...).callCount is incorrect 1. Update matchingFake function in spy.js Rename to matchingFakes, and return not last one fake (pop()) but all fakes matched. 2. Move matchingFakes function into spyApi Because of calling by stub.js 3. Modify spyApi.invoke function Apply processing at function invoked to all matching fakes. 4. Modify part of calling matchingFake() in spyApi.withArgs Fit logic, affected by 1. 5. Update proto.create.functionStub in stub.js Try to get the function stub using spyApi.matchingFakes() and pop(), affected by 1. --- lib/sinon/spy.js | 62 +++++++++++++++++++++++++++++------------------ lib/sinon/stub.js | 8 +++++- 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/lib/sinon/spy.js b/lib/sinon/spy.js index 91776e2f5..350c6aba2 100644 --- a/lib/sinon/spy.js +++ b/lib/sinon/spy.js @@ -41,18 +41,6 @@ function spy(object, property, types) { return wrapMethod(object, property, descriptor); } -function matchingFake(fakes, args, strict) { - if (!fakes) { - return undefined; - } - - var matchingFakes = fakes.filter(function (fake) { - return fake.matches(args, strict); - }); - - return matchingFakes.pop(); -} - function incrementCallCount() { this.called = true; this.callCount += 1; @@ -174,13 +162,22 @@ var spyApi = { }, invoke: function invoke(func, thisValue, args) { - var matching = matchingFake(this.fakes, args); - var exception, returnValue; + var matchings = this.matchingFakes(args); + var currentCallId = callId++; + var exception, returnValue, i; incrementCallCount.call(this); push.call(this.thisValues, thisValue); push.call(this.args, args); - push.call(this.callIds, callId++); + push.call(this.callIds, currentCallId); + if (matchings) { + for (i = 0; i < matchings.length; i++) { + incrementCallCount.call(matchings[i]); + push.call(matchings[i].thisValues, thisValue); + push.call(matchings[i].args, args); + push.call(matchings[i].callIds, currentCallId); + } + } // Make call properties available from within the spied function: createCallProperties.call(this); @@ -188,11 +185,7 @@ var spyApi = { try { this.invoking = true; - if (matching) { - returnValue = matching.invoke(func, thisValue, args); - } else { - returnValue = (this.func || func).apply(thisValue, args); - } + returnValue = (this.func || func).apply(thisValue, args); var thisCall = this.getCall(this.callCount - 1); if (thisCall.calledWithNew() && typeof returnValue !== "object") { @@ -206,6 +199,13 @@ var spyApi = { push.call(this.exceptions, exception); push.call(this.returnValues, returnValue); + if (matchings) { + for (i = 0; i < matchings.length; i++) { + push.call(matchings[i].exceptions, exception); + push.call(matchings[i].returnValues, returnValue); + } + } + var err = new ErrorConstructor(); // 1. Please do not get stack at this point. It may be so very slow, and not actually used // 2. PhantomJS does not serialize the stack trace until the error has been thrown: @@ -214,6 +214,11 @@ var spyApi = { throw err; } catch (e) {/* empty */} push.call(this.errorsWithCallStack, err); + if (matchings) { + for (i = 0; i < matchings.length; i++) { + push.call(matchings[i].errorsWithCallStack, err); + } + } // Make return value and exception available in the calls: createCallProperties.call(this); @@ -291,10 +296,11 @@ var spyApi = { var args = slice.call(arguments); if (this.fakes) { - var match = matchingFake(this.fakes, args, true); + var matchings = this.matchingFakes(args, true); + var matching = matchings && matchings.pop(); - if (match) { - return match; + if (matching) { + return matching; } } else { this.fakes = []; @@ -328,6 +334,16 @@ var spyApi = { return fake; }, + matchingFakes: function (args, strict) { + if (!this.fakes) { + return undefined; + } + + return this.fakes.filter(function (fake) { + return fake.matches(args, strict); + }); + }, + matches: function (args, strict) { var margs = this.matchingArguments; diff --git a/lib/sinon/stub.js b/lib/sinon/stub.js index ce7aa5213..4cae24c19 100644 --- a/lib/sinon/stub.js +++ b/lib/sinon/stub.js @@ -11,6 +11,8 @@ var stubEntireObject = require("./stub-entire-object"); var stubDescriptor = require("./stub-descriptor"); var throwOnFalsyObject = require("./throw-on-falsy-object"); +var slice = Array.prototype.slice; + function stub(object, property, descriptor) { throwOnFalsyObject.apply(null, arguments); @@ -85,7 +87,11 @@ var uuid = 0; var proto = { create: function create(stubLength) { var functionStub = function () { - return getCurrentBehavior(functionStub).invoke(this, arguments); + var args = slice.call(arguments); + var matchings = functionStub.matchingFakes(args); + + var fnStub = (matchings && matchings.pop()) || functionStub; + return getCurrentBehavior(fnStub).invoke(this, arguments); }; functionStub.id = "stub#" + uuid++; From 76ac773ae330a9169c7116d8469d51db53bcb173 Mon Sep 17 00:00:00 2001 From: takasmiley Date: Thu, 2 Mar 2017 12:51:19 +0900 Subject: [PATCH 3/8] Add test for #1274 case3 --- test/issues/issues-test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/issues/issues-test.js b/test/issues/issues-test.js index 7957f3c36..dbf4625cd 100644 --- a/test/issues/issues-test.js +++ b/test/issues/issues-test.js @@ -314,5 +314,17 @@ describe("issues", function () { assert.equals(spy.withArgs(1, 1).callCount, 1); assert.equals(spy.withArgs(1, 2).callCount, 1); }); + it("case3: should work on stub", function () { + var stub = sinon.stub(); + stub.returns(0); + stub.withArgs(1, 1).returns(2); + stub.withArgs(1).returns(1); + + assert.equals(stub(), 0); + assert.equals(stub(1), 1); + assert.equals(stub(1, 1), 2); + assert.equals(stub(1, 1, 1), 2); + assert.equals(stub(2), 0); + }); }); }); From f012fff1cdcef16228070c027ea8302ce1056615 Mon Sep 17 00:00:00 2001 From: takasmiley Date: Thu, 2 Mar 2017 12:53:25 +0900 Subject: [PATCH 4/8] Fix stub.withArgs(args...) is incorrect Change last pop to the most matched arguments at stub. --- lib/sinon/stub.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/sinon/stub.js b/lib/sinon/stub.js index 4cae24c19..88ad915c3 100644 --- a/lib/sinon/stub.js +++ b/lib/sinon/stub.js @@ -90,7 +90,15 @@ var proto = { var args = slice.call(arguments); var matchings = functionStub.matchingFakes(args); - var fnStub = (matchings && matchings.pop()) || functionStub; + var fnStub; + if (matchings) { + fnStub = matchings.sort(function (a, b) { + return a.matchingArguments.length - b.matchingArguments.length; + }).pop(); + } + if (!fnStub) { + fnStub = functionStub; + } return getCurrentBehavior(fnStub).invoke(this, arguments); }; From e72fee495d121a4fa3ad9a623a5a5a078412eab7 Mon Sep 17 00:00:00 2001 From: takasmiley Date: Fri, 3 Mar 2017 11:39:53 +0900 Subject: [PATCH 5/8] Update matchingFakes, always returns array --- lib/sinon/spy.js | 9 ++------- lib/sinon/stub.js | 12 +++--------- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/lib/sinon/spy.js b/lib/sinon/spy.js index 350c6aba2..6e2d193ed 100644 --- a/lib/sinon/spy.js +++ b/lib/sinon/spy.js @@ -296,8 +296,7 @@ var spyApi = { var args = slice.call(arguments); if (this.fakes) { - var matchings = this.matchingFakes(args, true); - var matching = matchings && matchings.pop(); + var matching = this.matchingFakes(args, true).pop(); if (matching) { return matching; @@ -335,11 +334,7 @@ var spyApi = { }, matchingFakes: function (args, strict) { - if (!this.fakes) { - return undefined; - } - - return this.fakes.filter(function (fake) { + return (this.fakes || []).filter(function (fake) { return fake.matches(args, strict); }); }, diff --git a/lib/sinon/stub.js b/lib/sinon/stub.js index 88ad915c3..1ed8dd320 100644 --- a/lib/sinon/stub.js +++ b/lib/sinon/stub.js @@ -90,15 +90,9 @@ var proto = { var args = slice.call(arguments); var matchings = functionStub.matchingFakes(args); - var fnStub; - if (matchings) { - fnStub = matchings.sort(function (a, b) { - return a.matchingArguments.length - b.matchingArguments.length; - }).pop(); - } - if (!fnStub) { - fnStub = functionStub; - } + var fnStub = matchings.sort(function (a, b) { + return a.matchingArguments.length - b.matchingArguments.length; + }).pop() || functionStub; return getCurrentBehavior(fnStub).invoke(this, arguments); }; From 833ff2c25f2a6af96b782422c1f1bef068d63bb2 Mon Sep 17 00:00:00 2001 From: takasmiley Date: Fri, 3 Mar 2017 11:40:52 +0900 Subject: [PATCH 6/8] Replace for loop with Array.prototype.forEach --- lib/sinon/spy.js | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/lib/sinon/spy.js b/lib/sinon/spy.js index 6e2d193ed..feab6e085 100644 --- a/lib/sinon/spy.js +++ b/lib/sinon/spy.js @@ -164,20 +164,18 @@ var spyApi = { invoke: function invoke(func, thisValue, args) { var matchings = this.matchingFakes(args); var currentCallId = callId++; - var exception, returnValue, i; + var exception, returnValue; incrementCallCount.call(this); push.call(this.thisValues, thisValue); push.call(this.args, args); push.call(this.callIds, currentCallId); - if (matchings) { - for (i = 0; i < matchings.length; i++) { - incrementCallCount.call(matchings[i]); - push.call(matchings[i].thisValues, thisValue); - push.call(matchings[i].args, args); - push.call(matchings[i].callIds, currentCallId); - } - } + matchings.forEach(function (matching) { + incrementCallCount.call(matching); + push.call(matching.thisValues, thisValue); + push.call(matching.args, args); + push.call(matching.callIds, currentCallId); + }); // Make call properties available from within the spied function: createCallProperties.call(this); @@ -199,12 +197,10 @@ var spyApi = { push.call(this.exceptions, exception); push.call(this.returnValues, returnValue); - if (matchings) { - for (i = 0; i < matchings.length; i++) { - push.call(matchings[i].exceptions, exception); - push.call(matchings[i].returnValues, returnValue); - } - } + matchings.forEach(function (matching) { + push.call(matching.exceptions, exception); + push.call(matching.returnValues, returnValue); + }); var err = new ErrorConstructor(); // 1. Please do not get stack at this point. It may be so very slow, and not actually used @@ -214,11 +210,9 @@ var spyApi = { throw err; } catch (e) {/* empty */} push.call(this.errorsWithCallStack, err); - if (matchings) { - for (i = 0; i < matchings.length; i++) { - push.call(matchings[i].errorsWithCallStack, err); - } - } + matchings.forEach(function (matching) { + push.call(matching.errorsWithCallStack, err); + }); // Make return value and exception available in the calls: createCallProperties.call(this); From cf3a34eafb6fdc230e5ad7f23fe5250c78c39014 Mon Sep 17 00:00:00 2001 From: takasmiley Date: Fri, 23 Jun 2017 11:14:12 +0900 Subject: [PATCH 7/8] Move #1274 test code to respective test files --- test/issues/issues-test.js | 57 -------------------------------------- test/spy-test.js | 39 ++++++++++++++++++++++++++ test/stub-test.js | 13 +++++++++ 3 files changed, 52 insertions(+), 57 deletions(-) diff --git a/test/issues/issues-test.js b/test/issues/issues-test.js index dbf4625cd..99461e547 100644 --- a/test/issues/issues-test.js +++ b/test/issues/issues-test.js @@ -270,61 +270,4 @@ describe("issues", function () { }); }); } - - describe("#1274 - spy.withArgs(args...).callCount is incorrect", function () { - it("case1: should count accurately", function () { - var obj = { - f: function () {} - }; - - // case1: withArgs(1) => withArgs(1, 1) - var spy = sinon.spy(obj, "f"); - assert.equals(spy.callCount, 0); - assert.equals(spy.withArgs(1).callCount, 0); - assert.equals(spy.withArgs(1, 1).callCount, 0); - - obj.f(); - obj.f(1); - obj.f(1, 1); - obj.f(1, 2); - - assert.equals(spy.callCount, 4); - assert.equals(spy.withArgs(1).callCount, 3); - assert.equals(spy.withArgs(1, 1).callCount, 1); - assert.equals(spy.withArgs(1, 2).callCount, 1); - }); - it("case2: should count accurately", function () { - var obj = { - f: function () {} - }; - - // case2: withArgs(1, 1) => withArgs(1) - var spy = sinon.spy(obj, "f"); - assert.equals(spy.callCount, 0); - assert.equals(spy.withArgs(1, 1).callCount, 0); - assert.equals(spy.withArgs(1).callCount, 0); - - obj.f(); - obj.f(1); - obj.f(1, 1); - obj.f(1, 2); - - assert.equals(spy.callCount, 4); - assert.equals(spy.withArgs(1).callCount, 3); - assert.equals(spy.withArgs(1, 1).callCount, 1); - assert.equals(spy.withArgs(1, 2).callCount, 1); - }); - it("case3: should work on stub", function () { - var stub = sinon.stub(); - stub.returns(0); - stub.withArgs(1, 1).returns(2); - stub.withArgs(1).returns(1); - - assert.equals(stub(), 0); - assert.equals(stub(1), 1); - assert.equals(stub(1, 1), 2); - assert.equals(stub(1, 1, 1), 2); - assert.equals(stub(2), 0); - }); - }); }); diff --git a/test/spy-test.js b/test/spy-test.js index 770929943..6c7ed90b4 100644 --- a/test/spy-test.js +++ b/test/spy-test.js @@ -326,6 +326,45 @@ describe("spy", function () { }); }); + it("counts with combination of withArgs arguments and order of calling withArgs", function () { + var object = { + f1: function () {}, + f2: function () {} + }; + + // f1: the order of withArgs(1), withArgs(1, 1) + var spy1 = createSpy(object, "f1"); + assert.equals(spy1.callCount, 0); + assert.equals(spy1.withArgs(1).callCount, 0); + assert.equals(spy1.withArgs(1, 1).callCount, 0); + + object.f1(); + object.f1(1); + object.f1(1, 1); + object.f1(1, 2); + + assert.equals(spy1.callCount, 4); + assert.equals(spy1.withArgs(1).callCount, 3); + assert.equals(spy1.withArgs(1, 1).callCount, 1); + assert.equals(spy1.withArgs(1, 2).callCount, 1); + + // f2: the order of withArgs(1, 1), withArgs(1) + var spy2 = createSpy(object, "f2"); + assert.equals(spy2.callCount, 0); + assert.equals(spy2.withArgs(1, 1).callCount, 0); + assert.equals(spy2.withArgs(1).callCount, 0); + + object.f2(); + object.f2(1); + object.f2(1, 1); + object.f2(1, 2); + + assert.equals(spy2.callCount, 4); + assert.equals(spy2.withArgs(1).callCount, 3); + assert.equals(spy2.withArgs(1, 1).callCount, 1); + assert.equals(spy2.withArgs(1, 2).callCount, 1); + }); + describe(".named", function () { it("sets displayName", function () { var spy = createSpy(); diff --git a/test/stub-test.js b/test/stub-test.js index b2a9cbb9f..3e16888a5 100644 --- a/test/stub-test.js +++ b/test/stub-test.js @@ -102,6 +102,19 @@ describe("stub", function () { assert(callback.called); }); + it("should works with combination of withArgs arguments", function () { + var stub = createStub(); + stub.returns(0); + stub.withArgs(1, 1).returns(2); + stub.withArgs(1).returns(1); + + assert.equals(stub(), 0); + assert.equals(stub(1), 1); + assert.equals(stub(1, 1), 2); + assert.equals(stub(1, 1, 1), 2); + assert.equals(stub(2), 0); + }); + describe(".returns", function () { it("returns specified value", function () { var stub = createStub.create(); From 1661a0f9eea1bcb03357a7a40af7948a50206cea Mon Sep 17 00:00:00 2001 From: takasmiley Date: Fri, 23 Jun 2017 15:05:48 +0900 Subject: [PATCH 8/8] Add test code of spy.matchingFakes --- test/spy-test.js | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/spy-test.js b/test/spy-test.js index 6c7ed90b4..9530627af 100644 --- a/test/spy-test.js +++ b/test/spy-test.js @@ -2507,5 +2507,44 @@ describe("spy", function () { assert.equals(spy.length, 3); }); }); + + describe(".matchingFakes", function () { + beforeEach(function () { + this.spy = createSpy(); + }); + + it("is function", function () { + assert.isFunction(this.spy.matchingFakes); + }); + + it("returns an empty array by default", function () { + assert.equals(this.spy.matchingFakes([]), []); + assert.equals(this.spy.matchingFakes([1]), []); + assert.equals(this.spy.matchingFakes([1, 1]), []); + }); + + it("returns one matched fake", function () { + this.spy.withArgs(1); + this.spy.withArgs(2); + + assert.equals(this.spy.matchingFakes([1]), [this.spy.withArgs(1)]); + assert.equals(this.spy.matchingFakes([2]), [this.spy.withArgs(2)]); + }); + + it("return some matched fake", function () { + this.spy.withArgs(1); + this.spy.withArgs(1, 1); + this.spy.withArgs(2); + + assert.equals(this.spy.matchingFakes([]), []); + assert.equals(this.spy.matchingFakes([1]), [ + this.spy.withArgs(1) + ]); + assert.equals(this.spy.matchingFakes([1, 1]), [ + this.spy.withArgs(1), + this.spy.withArgs(1, 1) + ]); + }); + }); });