From 9fa496b0298a9ead260070bc548291f571840c70 Mon Sep 17 00:00:00 2001 From: Erick Wendel Date: Sat, 17 Dec 2022 11:35:09 -0300 Subject: [PATCH] test: fix mock.method to support class instances It fixes a problem when trying to spy a method from a class instance or static functions on a class instance PR-URL: https://github.com/nodejs/node/pull/45608 Reviewed-By: Colin Ihrig Reviewed-By: Yagiz Nizipli Reviewed-By: Benjamin Gruenbaum Reviewed-By: Antoine du Hamel (cherry picked from commit 929aada39d0f418193ca03cc360ced8c5b4ce553) --- lib/internal/per_context/primordials.js | 1 + lib/internal/test_runner/mock.js | 32 ++++- test/parallel/test-runner-mocking.js | 152 +++++++++++++++++++++++- 3 files changed, 178 insertions(+), 7 deletions(-) diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js index 951bf28..2f12a34 100644 --- a/lib/internal/per_context/primordials.js +++ b/lib/internal/per_context/primordials.js @@ -40,6 +40,7 @@ exports.ObjectDefineProperty = (obj, key, descr) => Object.defineProperty(obj, k exports.ObjectEntries = obj => Object.entries(obj) exports.ObjectFreeze = obj => Object.freeze(obj) exports.ObjectGetOwnPropertyDescriptor = (obj, key) => Object.getOwnPropertyDescriptor(obj, key) +exports.ObjectGetPrototypeOf = obj => Object.getPrototypeOf(obj) exports.ObjectIsExtensible = obj => Object.isExtensible(obj) exports.ObjectPrototypeHasOwnProperty = (obj, property) => Object.prototype.hasOwnProperty.call(obj, property) exports.ObjectSeal = (obj) => Object.seal(obj) diff --git a/lib/internal/test_runner/mock.js b/lib/internal/test_runner/mock.js index 9959825..af46e47 100644 --- a/lib/internal/test_runner/mock.js +++ b/lib/internal/test_runner/mock.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/afed1afa55962211b6b56c2068d520b4d8a08888/lib/internal/test_runner/mock.js +// https://github.com/nodejs/node/blob/929aada39d0f418193ca03cc360ced8c5b4ce553/lib/internal/test_runner/mock.js 'use strict' const { ArrayPrototypePush, @@ -7,6 +7,7 @@ const { FunctionPrototypeCall, ObjectDefineProperty, ObjectGetOwnPropertyDescriptor, + ObjectGetPrototypeOf, Proxy, ReflectApply, ReflectConstruct, @@ -127,13 +128,15 @@ class MockTracker { } method ( - object, + objectOrFunction, methodName, implementation = kDefaultFunction, options = kEmptyObject ) { - validateObject(object, 'object') validateStringOrSymbol(methodName, 'methodName') + if (typeof objectOrFunction !== 'function') { + validateObject(objectOrFunction, 'object') + } if (implementation !== null && typeof implementation === 'object') { options = implementation @@ -158,8 +161,8 @@ class MockTracker { 'options.setter', setter, "cannot be used with 'options.getter'" ) } + const descriptor = findMethodOnPrototypeChain(objectOrFunction, methodName) - const descriptor = ObjectGetOwnPropertyDescriptor(object, methodName) let original if (getter) { @@ -176,7 +179,7 @@ class MockTracker { ) } - const restore = { descriptor, object, methodName } + const restore = { descriptor, object: objectOrFunction, methodName } const impl = implementation === kDefaultFunction ? original : implementation @@ -199,7 +202,7 @@ class MockTracker { mockDescriptor.value = mock } - ObjectDefineProperty(object, methodName, mockDescriptor) + ObjectDefineProperty(objectOrFunction, methodName, mockDescriptor) return mock } @@ -348,4 +351,21 @@ function validateTimes (value, name) { validateInteger(value, name, 1) } +function findMethodOnPrototypeChain (instance, methodName) { + let host = instance + let descriptor + + while (host !== null) { + descriptor = ObjectGetOwnPropertyDescriptor(host, methodName) + + if (descriptor) { + break + } + + host = ObjectGetPrototypeOf(host) + } + + return descriptor +} + module.exports = { MockTracker } diff --git a/test/parallel/test-runner-mocking.js b/test/parallel/test-runner-mocking.js index 6bcff47..f956c52 100644 --- a/test/parallel/test-runner-mocking.js +++ b/test/parallel/test-runner-mocking.js @@ -1,4 +1,4 @@ -// https://github.com/nodejs/node/blob/afed1afa55962211b6b56c2068d520b4d8a08888/test/parallel/test-runner-mocking.js +// https://github.com/nodejs/node/blob/929aada39d0f418193ca03cc360ced8c5b4ce553/test/parallel/test-runner-mocking.js 'use strict' const common = require('../common') const assert = require('node:assert') @@ -320,6 +320,156 @@ test('spy functions can be bound', (t) => { assert.strictEqual(sum.bind(0)(2, 11), 13) }) +test('mocks prototype methods on an instance', async (t) => { + class Runner { + async someTask (msg) { + return Promise.resolve(msg) + } + + async method (msg) { + await this.someTask(msg) + return msg + } + } + const msg = 'ok' + const obj = new Runner() + assert.strictEqual(await obj.method(msg), msg) + + t.mock.method(obj, obj.someTask.name) + assert.strictEqual(obj.someTask.mock.calls.length, 0) + + assert.strictEqual(await obj.method(msg), msg) + + const call = obj.someTask.mock.calls[0] + + assert.deepStrictEqual(call.arguments, [msg]) + assert.strictEqual(await call.result, msg) + assert.strictEqual(call.target, undefined) + assert.strictEqual(call.this, obj) + + const obj2 = new Runner() + // Ensure that a brand new instance is not mocked + assert.strictEqual( + obj2.someTask.mock, + undefined + ) + + assert.strictEqual(obj.someTask.mock.restore(), undefined) + assert.strictEqual(await obj.method(msg), msg) + assert.strictEqual(obj.someTask.mock, undefined) + assert.strictEqual(Runner.prototype.someTask.mock, undefined) +}) + +test('spies on async static class methods', async (t) => { + class Runner { + static async someTask (msg) { + return Promise.resolve(msg) + } + + static async method (msg) { + await this.someTask(msg) + return msg + } + } + const msg = 'ok' + assert.strictEqual(await Runner.method(msg), msg) + + t.mock.method(Runner, Runner.someTask.name) + assert.strictEqual(Runner.someTask.mock.calls.length, 0) + + assert.strictEqual(await Runner.method(msg), msg) + + const call = Runner.someTask.mock.calls[0] + + assert.deepStrictEqual(call.arguments, [msg]) + assert.strictEqual(await call.result, msg) + assert.strictEqual(call.target, undefined) + assert.strictEqual(call.this, Runner) + + assert.strictEqual(Runner.someTask.mock.restore(), undefined) + assert.strictEqual(await Runner.method(msg), msg) + assert.strictEqual(Runner.someTask.mock, undefined) + assert.strictEqual(Runner.prototype.someTask, undefined) +}) + +test('given null to a mock.method it throws a invalid argument error', (t) => { + assert.throws(() => t.mock.method(null, {}), { code: 'ERR_INVALID_ARG_TYPE' }) +}) + +test('it should throw given an inexistent property on a object instance', (t) => { + assert.throws(() => t.mock.method({ abc: 0 }, 'non-existent'), { + code: 'ERR_INVALID_ARG_VALUE' + }) +}) + +test('spy functions can be used on classes inheritance', (t) => { + // Makes sure that having a null-prototype doesn't throw our system off + class A extends null { + static someTask (msg) { + return msg + } + + static method (msg) { + return this.someTask(msg) + } + } + class B extends A {} + class C extends B {} + + const msg = 'ok' + assert.strictEqual(C.method(msg), msg) + + t.mock.method(C, C.someTask.name) + assert.strictEqual(C.someTask.mock.calls.length, 0) + + assert.strictEqual(C.method(msg), msg) + + const call = C.someTask.mock.calls[0] + + assert.deepStrictEqual(call.arguments, [msg]) + assert.strictEqual(call.result, msg) + assert.strictEqual(call.target, undefined) + assert.strictEqual(call.this, C) + + assert.strictEqual(C.someTask.mock.restore(), undefined) + assert.strictEqual(C.method(msg), msg) + assert.strictEqual(C.someTask.mock, undefined) +}) + +test('spy functions don\'t affect the prototype chain', (t) => { + class A { + static someTask (msg) { + return msg + } + } + class B extends A {} + class C extends B {} + + const msg = 'ok' + + const ABeforeMockIsUnchanged = Object.getOwnPropertyDescriptor(A, A.someTask.name) + const BBeforeMockIsUnchanged = Object.getOwnPropertyDescriptor(B, B.someTask.name) + const CBeforeMockShouldNotHaveDesc = Object.getOwnPropertyDescriptor(C, C.someTask.name) + + t.mock.method(C, C.someTask.name) + C.someTask(msg) + const BAfterMockIsUnchanged = Object.getOwnPropertyDescriptor(B, B.someTask.name) + + const AAfterMockIsUnchanged = Object.getOwnPropertyDescriptor(A, A.someTask.name) + const CAfterMockHasDescriptor = Object.getOwnPropertyDescriptor(C, C.someTask.name) + + assert.strictEqual(CBeforeMockShouldNotHaveDesc, undefined) + assert.ok(CAfterMockHasDescriptor) + + assert.deepStrictEqual(ABeforeMockIsUnchanged, AAfterMockIsUnchanged) + assert.strictEqual(BBeforeMockIsUnchanged, BAfterMockIsUnchanged) + assert.strictEqual(BBeforeMockIsUnchanged, undefined) + + assert.strictEqual(C.someTask.mock.restore(), undefined) + const CAfterRestoreKeepsDescriptor = Object.getOwnPropertyDescriptor(C, C.someTask.name) + assert.ok(CAfterRestoreKeepsDescriptor) +}) + test('mocked functions report thrown errors', (t) => { const testError = new Error('test error') const fn = t.mock.fn(() => {