Skip to content

Commit 9fa496b

Browse files
ErickWendelMoLow
authored andcommitted
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: nodejs/node#45608 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> (cherry picked from commit 929aada39d0f418193ca03cc360ced8c5b4ce553)
1 parent c80e426 commit 9fa496b

File tree

3 files changed

+178
-7
lines changed

3 files changed

+178
-7
lines changed

lib/internal/per_context/primordials.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ exports.ObjectDefineProperty = (obj, key, descr) => Object.defineProperty(obj, k
4040
exports.ObjectEntries = obj => Object.entries(obj)
4141
exports.ObjectFreeze = obj => Object.freeze(obj)
4242
exports.ObjectGetOwnPropertyDescriptor = (obj, key) => Object.getOwnPropertyDescriptor(obj, key)
43+
exports.ObjectGetPrototypeOf = obj => Object.getPrototypeOf(obj)
4344
exports.ObjectIsExtensible = obj => Object.isExtensible(obj)
4445
exports.ObjectPrototypeHasOwnProperty = (obj, property) => Object.prototype.hasOwnProperty.call(obj, property)
4546
exports.ObjectSeal = (obj) => Object.seal(obj)

lib/internal/test_runner/mock.js

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// https://github.com/nodejs/node/blob/afed1afa55962211b6b56c2068d520b4d8a08888/lib/internal/test_runner/mock.js
1+
// https://github.com/nodejs/node/blob/929aada39d0f418193ca03cc360ced8c5b4ce553/lib/internal/test_runner/mock.js
22
'use strict'
33
const {
44
ArrayPrototypePush,
@@ -7,6 +7,7 @@ const {
77
FunctionPrototypeCall,
88
ObjectDefineProperty,
99
ObjectGetOwnPropertyDescriptor,
10+
ObjectGetPrototypeOf,
1011
Proxy,
1112
ReflectApply,
1213
ReflectConstruct,
@@ -127,13 +128,15 @@ class MockTracker {
127128
}
128129

129130
method (
130-
object,
131+
objectOrFunction,
131132
methodName,
132133
implementation = kDefaultFunction,
133134
options = kEmptyObject
134135
) {
135-
validateObject(object, 'object')
136136
validateStringOrSymbol(methodName, 'methodName')
137+
if (typeof objectOrFunction !== 'function') {
138+
validateObject(objectOrFunction, 'object')
139+
}
137140

138141
if (implementation !== null && typeof implementation === 'object') {
139142
options = implementation
@@ -158,8 +161,8 @@ class MockTracker {
158161
'options.setter', setter, "cannot be used with 'options.getter'"
159162
)
160163
}
164+
const descriptor = findMethodOnPrototypeChain(objectOrFunction, methodName)
161165

162-
const descriptor = ObjectGetOwnPropertyDescriptor(object, methodName)
163166
let original
164167

165168
if (getter) {
@@ -176,7 +179,7 @@ class MockTracker {
176179
)
177180
}
178181

179-
const restore = { descriptor, object, methodName }
182+
const restore = { descriptor, object: objectOrFunction, methodName }
180183
const impl = implementation === kDefaultFunction
181184
? original
182185
: implementation
@@ -199,7 +202,7 @@ class MockTracker {
199202
mockDescriptor.value = mock
200203
}
201204

202-
ObjectDefineProperty(object, methodName, mockDescriptor)
205+
ObjectDefineProperty(objectOrFunction, methodName, mockDescriptor)
203206

204207
return mock
205208
}
@@ -348,4 +351,21 @@ function validateTimes (value, name) {
348351
validateInteger(value, name, 1)
349352
}
350353

354+
function findMethodOnPrototypeChain (instance, methodName) {
355+
let host = instance
356+
let descriptor
357+
358+
while (host !== null) {
359+
descriptor = ObjectGetOwnPropertyDescriptor(host, methodName)
360+
361+
if (descriptor) {
362+
break
363+
}
364+
365+
host = ObjectGetPrototypeOf(host)
366+
}
367+
368+
return descriptor
369+
}
370+
351371
module.exports = { MockTracker }

test/parallel/test-runner-mocking.js

Lines changed: 151 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// https://github.com/nodejs/node/blob/afed1afa55962211b6b56c2068d520b4d8a08888/test/parallel/test-runner-mocking.js
1+
// https://github.com/nodejs/node/blob/929aada39d0f418193ca03cc360ced8c5b4ce553/test/parallel/test-runner-mocking.js
22
'use strict'
33
const common = require('../common')
44
const assert = require('node:assert')
@@ -320,6 +320,156 @@ test('spy functions can be bound', (t) => {
320320
assert.strictEqual(sum.bind(0)(2, 11), 13)
321321
})
322322

323+
test('mocks prototype methods on an instance', async (t) => {
324+
class Runner {
325+
async someTask (msg) {
326+
return Promise.resolve(msg)
327+
}
328+
329+
async method (msg) {
330+
await this.someTask(msg)
331+
return msg
332+
}
333+
}
334+
const msg = 'ok'
335+
const obj = new Runner()
336+
assert.strictEqual(await obj.method(msg), msg)
337+
338+
t.mock.method(obj, obj.someTask.name)
339+
assert.strictEqual(obj.someTask.mock.calls.length, 0)
340+
341+
assert.strictEqual(await obj.method(msg), msg)
342+
343+
const call = obj.someTask.mock.calls[0]
344+
345+
assert.deepStrictEqual(call.arguments, [msg])
346+
assert.strictEqual(await call.result, msg)
347+
assert.strictEqual(call.target, undefined)
348+
assert.strictEqual(call.this, obj)
349+
350+
const obj2 = new Runner()
351+
// Ensure that a brand new instance is not mocked
352+
assert.strictEqual(
353+
obj2.someTask.mock,
354+
undefined
355+
)
356+
357+
assert.strictEqual(obj.someTask.mock.restore(), undefined)
358+
assert.strictEqual(await obj.method(msg), msg)
359+
assert.strictEqual(obj.someTask.mock, undefined)
360+
assert.strictEqual(Runner.prototype.someTask.mock, undefined)
361+
})
362+
363+
test('spies on async static class methods', async (t) => {
364+
class Runner {
365+
static async someTask (msg) {
366+
return Promise.resolve(msg)
367+
}
368+
369+
static async method (msg) {
370+
await this.someTask(msg)
371+
return msg
372+
}
373+
}
374+
const msg = 'ok'
375+
assert.strictEqual(await Runner.method(msg), msg)
376+
377+
t.mock.method(Runner, Runner.someTask.name)
378+
assert.strictEqual(Runner.someTask.mock.calls.length, 0)
379+
380+
assert.strictEqual(await Runner.method(msg), msg)
381+
382+
const call = Runner.someTask.mock.calls[0]
383+
384+
assert.deepStrictEqual(call.arguments, [msg])
385+
assert.strictEqual(await call.result, msg)
386+
assert.strictEqual(call.target, undefined)
387+
assert.strictEqual(call.this, Runner)
388+
389+
assert.strictEqual(Runner.someTask.mock.restore(), undefined)
390+
assert.strictEqual(await Runner.method(msg), msg)
391+
assert.strictEqual(Runner.someTask.mock, undefined)
392+
assert.strictEqual(Runner.prototype.someTask, undefined)
393+
})
394+
395+
test('given null to a mock.method it throws a invalid argument error', (t) => {
396+
assert.throws(() => t.mock.method(null, {}), { code: 'ERR_INVALID_ARG_TYPE' })
397+
})
398+
399+
test('it should throw given an inexistent property on a object instance', (t) => {
400+
assert.throws(() => t.mock.method({ abc: 0 }, 'non-existent'), {
401+
code: 'ERR_INVALID_ARG_VALUE'
402+
})
403+
})
404+
405+
test('spy functions can be used on classes inheritance', (t) => {
406+
// Makes sure that having a null-prototype doesn't throw our system off
407+
class A extends null {
408+
static someTask (msg) {
409+
return msg
410+
}
411+
412+
static method (msg) {
413+
return this.someTask(msg)
414+
}
415+
}
416+
class B extends A {}
417+
class C extends B {}
418+
419+
const msg = 'ok'
420+
assert.strictEqual(C.method(msg), msg)
421+
422+
t.mock.method(C, C.someTask.name)
423+
assert.strictEqual(C.someTask.mock.calls.length, 0)
424+
425+
assert.strictEqual(C.method(msg), msg)
426+
427+
const call = C.someTask.mock.calls[0]
428+
429+
assert.deepStrictEqual(call.arguments, [msg])
430+
assert.strictEqual(call.result, msg)
431+
assert.strictEqual(call.target, undefined)
432+
assert.strictEqual(call.this, C)
433+
434+
assert.strictEqual(C.someTask.mock.restore(), undefined)
435+
assert.strictEqual(C.method(msg), msg)
436+
assert.strictEqual(C.someTask.mock, undefined)
437+
})
438+
439+
test('spy functions don\'t affect the prototype chain', (t) => {
440+
class A {
441+
static someTask (msg) {
442+
return msg
443+
}
444+
}
445+
class B extends A {}
446+
class C extends B {}
447+
448+
const msg = 'ok'
449+
450+
const ABeforeMockIsUnchanged = Object.getOwnPropertyDescriptor(A, A.someTask.name)
451+
const BBeforeMockIsUnchanged = Object.getOwnPropertyDescriptor(B, B.someTask.name)
452+
const CBeforeMockShouldNotHaveDesc = Object.getOwnPropertyDescriptor(C, C.someTask.name)
453+
454+
t.mock.method(C, C.someTask.name)
455+
C.someTask(msg)
456+
const BAfterMockIsUnchanged = Object.getOwnPropertyDescriptor(B, B.someTask.name)
457+
458+
const AAfterMockIsUnchanged = Object.getOwnPropertyDescriptor(A, A.someTask.name)
459+
const CAfterMockHasDescriptor = Object.getOwnPropertyDescriptor(C, C.someTask.name)
460+
461+
assert.strictEqual(CBeforeMockShouldNotHaveDesc, undefined)
462+
assert.ok(CAfterMockHasDescriptor)
463+
464+
assert.deepStrictEqual(ABeforeMockIsUnchanged, AAfterMockIsUnchanged)
465+
assert.strictEqual(BBeforeMockIsUnchanged, BAfterMockIsUnchanged)
466+
assert.strictEqual(BBeforeMockIsUnchanged, undefined)
467+
468+
assert.strictEqual(C.someTask.mock.restore(), undefined)
469+
const CAfterRestoreKeepsDescriptor = Object.getOwnPropertyDescriptor(C, C.someTask.name)
470+
assert.ok(CAfterRestoreKeepsDescriptor)
471+
})
472+
323473
test('mocked functions report thrown errors', (t) => {
324474
const testError = new Error('test error')
325475
const fn = t.mock.fn(() => {

0 commit comments

Comments
 (0)