From 187b99bfe009d6cb96201dc6e438ddf26818cbbb Mon Sep 17 00:00:00 2001 From: OneNail Date: Sat, 7 May 2022 06:00:53 +0800 Subject: [PATCH] assert: fix CallTracker wraps the function causes the length to be lost PR-URL: https://github.com/nodejs/node/pull/42909 Fixes: https://github.com/nodejs/node/issues/40484 Reviewed-By: Antoine du Hamel --- lib/internal/assert/calltracker.js | 32 ++++++------ .../parallel/test-assert-calltracker-calls.js | 50 ++++++++++++++++++- 2 files changed, 67 insertions(+), 15 deletions(-) diff --git a/lib/internal/assert/calltracker.js b/lib/internal/assert/calltracker.js index 0fbdf70e5d825c..f00f2e33271980 100644 --- a/lib/internal/assert/calltracker.js +++ b/lib/internal/assert/calltracker.js @@ -4,6 +4,7 @@ const { ArrayPrototypePush, Error, FunctionPrototype, + Proxy, ReflectApply, SafeSet, } = primordials; @@ -46,20 +47,23 @@ class CallTracker { const callChecks = this.#callChecks; callChecks.add(context); - return function() { - 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(fn, this, arguments); - }; + return new Proxy(fn, { + __proto__: null, + apply(fn, thisArg, argList) { + 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(fn, thisArg, argList); + }, + }); } report() { diff --git a/test/parallel/test-assert-calltracker-calls.js b/test/parallel/test-assert-calltracker-calls.js index 99db4ee284be81..7b73f3fefaf6ab 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. @@ -78,3 +78,51 @@ assert.throws( callsNoop(); tracker.verify(); } + +{ + function func() {} + const tracker = new assert.CallTracker(); + const callsfunc = tracker.calls(func); + assert.strictEqual(callsfunc.length, 0); +} + +{ + function func(a, b, c = 2) {} + const tracker = new assert.CallTracker(); + const callsfunc = tracker.calls(func); + 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); +} + +{ + 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) { + 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; +}