assert: fix CallTracker wraps the function causes the length to be lost

PR-URL: #42909 Fixes: #40484 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
nodejs · May 6, 2022 · 187b99b · 187b99b
1 parent 1af7054
commit 187b99b
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 15 deletions.
diff --git 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
@@ -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;
+}