From 91efcf612ea5d26ecb0bfc5c7d4060183002f902 Mon Sep 17 00:00:00 2001 From: OneNail Date: Fri, 29 Apr 2022 21:49:01 +0800 Subject: [PATCH 1/5] assert: fix CallTracker wraps the function causes the length to be lost --- lib/internal/assert/calltracker.js | 11 +++++++++-- test/parallel/test-assert-calltracker-calls.js | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/internal/assert/calltracker.js b/lib/internal/assert/calltracker.js index 0fbdf70e5d825c..6ad1ffc7f16207 100644 --- a/lib/internal/assert/calltracker.js +++ b/lib/internal/assert/calltracker.js @@ -4,6 +4,7 @@ const { ArrayPrototypePush, Error, FunctionPrototype, + ObjectDefineProperty, ReflectApply, SafeSet, } = primordials; @@ -46,7 +47,7 @@ class CallTracker { const callChecks = this.#callChecks; callChecks.add(context); - return function() { + function callsFn() { context.actual++; if (context.actual === context.exact) { // Once function has reached its call count remove it from @@ -59,7 +60,13 @@ class CallTracker { callChecks.add(context); } return ReflectApply(fn, this, arguments); - }; + } + + ObjectDefineProperty(callsFn, 'length', { + value: fn.length + }); + + return callsFn; } report() { diff --git a/test/parallel/test-assert-calltracker-calls.js b/test/parallel/test-assert-calltracker-calls.js index 99db4ee284be81..f190f053a85c1e 100644 --- a/test/parallel/test-assert-calltracker-calls.js +++ b/test/parallel/test-assert-calltracker-calls.js @@ -78,3 +78,17 @@ assert.throws( callsNoop(); tracker.verify(); } + +{ + function func() {} + const tracker = new assert.CallTracker(); + const callsfunc = tracker.calls(func); + assert.strictEqual(func.length, callsfunc.length); +} + +{ + function func(a, b, c = 2) {} + const tracker = new assert.CallTracker(); + const callsfunc = tracker.calls(func); + assert.strictEqual(func.length, callsfunc.length); +} From 8da26f6eb8060441de24dc5c9be1ef9e2330c8e5 Mon Sep 17 00:00:00 2001 From: OneNail Date: Sat, 30 Apr 2022 00:19:48 +0800 Subject: [PATCH 2/5] assert: use proxy in calltracker calls --- lib/internal/assert/calltracker.js | 34 ++++++++----------- .../parallel/test-assert-calltracker-calls.js | 22 ++++++++++-- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/lib/internal/assert/calltracker.js b/lib/internal/assert/calltracker.js index 6ad1ffc7f16207..390dddaed64c85 100644 --- a/lib/internal/assert/calltracker.js +++ b/lib/internal/assert/calltracker.js @@ -4,7 +4,7 @@ const { ArrayPrototypePush, Error, FunctionPrototype, - ObjectDefineProperty, + Proxy, ReflectApply, SafeSet, } = primordials; @@ -47,26 +47,22 @@ class CallTracker { const callChecks = this.#callChecks; callChecks.add(context); - function callsFn() { - context.actual++; - if (context.actual === context.exact) { - // Once function has reached its call count remove it from - // callChecks set to prevent memory leaks. - callChecks.delete(context); + return new Proxy(fn, { + apply() { + context.actual++; + if (context.actual === context.exact) { + // Once function has reached its call count remove it from + // callChecks set to prevent memory leaks. + callChecks.delete(context); + } + // If function has been called more than expected times, add back into + // callchecks. + if (context.actual === context.exact + 1) { + callChecks.add(context); + } + return ReflectApply(...arguments); } - // If function has been called more than expected times, add back into - // callchecks. - if (context.actual === context.exact + 1) { - callChecks.add(context); - } - return ReflectApply(fn, this, arguments); - } - - ObjectDefineProperty(callsFn, 'length', { - value: fn.length }); - - return callsFn; } report() { diff --git a/test/parallel/test-assert-calltracker-calls.js b/test/parallel/test-assert-calltracker-calls.js index f190f053a85c1e..69ef8845a5e36c 100644 --- a/test/parallel/test-assert-calltracker-calls.js +++ b/test/parallel/test-assert-calltracker-calls.js @@ -1,5 +1,5 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); // This test ensures that assert.CallTracker.calls() works as intended. @@ -83,12 +83,28 @@ assert.throws( function func() {} const tracker = new assert.CallTracker(); const callsfunc = tracker.calls(func); - assert.strictEqual(func.length, callsfunc.length); + assert.strictEqual(callsfunc.length, 0); } { function func(a, b, c = 2) {} const tracker = new assert.CallTracker(); const callsfunc = tracker.calls(func); - assert.strictEqual(func.length, callsfunc.length); + assert.strictEqual(callsfunc.length, 2); +} + +{ + function func(a, b, c = 2) {} + delete func.length; + const tracker = new assert.CallTracker(); + const callsfunc = tracker.calls(func); + assert.strictEqual(Object.hasOwn(callsfunc, 'length'), false); +} + +{ + function func(a, b, c = 2) {} + Object.defineProperty(func, 'length', { get: common.mustNotCall() }); + const tracker = new assert.CallTracker(); + const callsfunc = tracker.calls(func); + assert.strictEqual(Object.hasOwn(callsfunc, 'length'), true); } From 4cde699e1272311418666418469584525e41eaf0 Mon Sep 17 00:00:00 2001 From: OneNail Date: Sat, 30 Apr 2022 01:10:12 +0800 Subject: [PATCH 3/5] assert: calltracker calls edge case --- test/parallel/test-assert-calltracker-calls.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/parallel/test-assert-calltracker-calls.js b/test/parallel/test-assert-calltracker-calls.js index 69ef8845a5e36c..ca028cd82c5a05 100644 --- a/test/parallel/test-assert-calltracker-calls.js +++ b/test/parallel/test-assert-calltracker-calls.js @@ -102,9 +102,15 @@ assert.throws( } { + Object.prototype.get = common.mustNotCall('%Object.prototype%.get'); + const customPropertyValue = Symbol(); function func(a, b, c = 2) {} + func.customProperty = customPropertyValue; Object.defineProperty(func, 'length', { get: common.mustNotCall() }); const tracker = new assert.CallTracker(); const callsfunc = tracker.calls(func); assert.strictEqual(Object.hasOwn(callsfunc, 'length'), true); + assert.strictEqual(callsfunc.customProperty, customPropertyValue); + + delete Object.prototype.get; } From 6820851a00db904e24ec4432447b0be5a9873fe4 Mon Sep 17 00:00:00 2001 From: OneNail Date: Sat, 30 Apr 2022 03:01:53 +0800 Subject: [PATCH 4/5] assert: set __proto__ null in calltracker calls proxy --- lib/internal/assert/calltracker.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/assert/calltracker.js b/lib/internal/assert/calltracker.js index 390dddaed64c85..7d586f42922a91 100644 --- a/lib/internal/assert/calltracker.js +++ b/lib/internal/assert/calltracker.js @@ -48,6 +48,7 @@ class CallTracker { callChecks.add(context); return new Proxy(fn, { + __proto__: null, apply() { context.actual++; if (context.actual === context.exact) { @@ -61,7 +62,7 @@ class CallTracker { callChecks.add(context); } return ReflectApply(...arguments); - } + }, }); } From 818a5a5bb4adeabdf098e49c0c06d64bb8fc0970 Mon Sep 17 00:00:00 2001 From: OneNail Date: Wed, 4 May 2022 01:51:38 +0800 Subject: [PATCH 5/5] assert: calls avoid ArrayIteratorPrototype next changed --- lib/internal/assert/calltracker.js | 4 ++-- test/parallel/test-assert-calltracker-calls.js | 14 +++++++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/internal/assert/calltracker.js b/lib/internal/assert/calltracker.js index 7d586f42922a91..f00f2e33271980 100644 --- a/lib/internal/assert/calltracker.js +++ b/lib/internal/assert/calltracker.js @@ -49,7 +49,7 @@ class CallTracker { return new Proxy(fn, { __proto__: null, - apply() { + apply(fn, thisArg, argList) { context.actual++; if (context.actual === context.exact) { // Once function has reached its call count remove it from @@ -61,7 +61,7 @@ class CallTracker { if (context.actual === context.exact + 1) { callChecks.add(context); } - return ReflectApply(...arguments); + return ReflectApply(fn, thisArg, argList); }, }); } diff --git a/test/parallel/test-assert-calltracker-calls.js b/test/parallel/test-assert-calltracker-calls.js index ca028cd82c5a05..7b73f3fefaf6ab 100644 --- a/test/parallel/test-assert-calltracker-calls.js +++ b/test/parallel/test-assert-calltracker-calls.js @@ -102,15 +102,27 @@ assert.throws( } { + const ArrayIteratorPrototype = Reflect.getPrototypeOf( + Array.prototype.values() + ); + const { next } = ArrayIteratorPrototype; + ArrayIteratorPrototype.next = common.mustNotCall( + '%ArrayIteratorPrototype%.next' + ); Object.prototype.get = common.mustNotCall('%Object.prototype%.get'); + const customPropertyValue = Symbol(); - function func(a, b, c = 2) {} + function func(a, b, c = 2) { + return a + b + c; + } func.customProperty = customPropertyValue; Object.defineProperty(func, 'length', { get: common.mustNotCall() }); const tracker = new assert.CallTracker(); const callsfunc = tracker.calls(func); assert.strictEqual(Object.hasOwn(callsfunc, 'length'), true); assert.strictEqual(callsfunc.customProperty, customPropertyValue); + assert.strictEqual(callsfunc(1, 2, 3), 6); + ArrayIteratorPrototype.next = next; delete Object.prototype.get; }