drashland / rhum

A test double library
https://drash.land/rhum
MIT License
92 stars 6 forks source link

Rhum.mock doesn't track nested method calls #136

Closed crookse closed 2 years ago

crookse commented 2 years ago

Summary

See comment from Discord:

Screen Shot 2022-02-21 at 7 53 02 PM

Root Cause

The issue is with this block of code in mock_builder.ts#create():

        mock[method] = function (...args: unknown[]) {
          mock.calls[method]++;
          return (original[method as keyof T] as unknown as (
            ...params: unknown[]
          ) => unknown)(...args);
        };

The above code turns every method in an object into a method that can track itself. For example:

add(number: string) { ... }

becomes

function(...args: unknown) {
  mock.calls["add"]++;
  return add(...args);
}

In the problematic block, there is this:

          return (original[method as keyof T] as unknown as (
            ...params: unknown[]
          ) => unknown)(...args);

This makes each method being mocked call the ORIGINAL object's method. So, what's happening in this issue is:

  1. <real object>.add() is replaced with the wrapper function that can track calls via mock.calls[method]++
  2. mock.add() is called
  3. mock.add() calls mock.calls["add"]++ and mock.calls.add === 1 now
  4. mock.add() calls the original version of itself
  5. the original version of mock.add() calls the original version of realAdd()
  6. the original version of realAdd() doesn't have mock.calls["realAdd"]++, so mock.calls.realAdd === 0

Example test code

import { Rhum } from "https://deno.land/x/rhum@v1.1.12/mod.ts";

class MathService {
  realAdd(num1: number, num2: number): number {
    return num1 + num2;
  }
  add(num1: number, num2: number): number {
    return this.realAdd(num1, num2);
  }
}

const mock = Rhum.mock(MathService).create();

Rhum.asserts.assertEquals(mock.calls.realAdd, 0);

// Since this calls `.readAdd()`, we expect `mock.calls.realAdd` to be `1` below, but it is not
mock.add(1, 1);

// Should be 1, not 0
Rhum.asserts.assertEquals(mock.calls.realAdd, 1);

Rhum.run()
stav commented 2 years ago

I can confirm this is working now.

crookse commented 2 years ago

thanks for confirming @stav ! also, thank you for bringing this up!

stav commented 2 years ago

Well I sure do love working on actively maintained software for just these kinds of contributions.

I actually did try and find the bug in the code but it was a bit over my head.

So I did this:

Rhum.testSuite("check", () => {

  let calls
  let xsocketMock

  class XapiSocketMock {
    check = check // code being tested
    trades = () => { calls.trades++ }
  }

  Rhum.testCase("should call trades once", async () => {
    calls = { trades: 0 }
    xsocketMock = new XapiSocketMock()
    await xsocketMock.check(data)
    Rhum.asserts.assertEquals( calls.trades.length, 1 )
  })

})

But now with your fix I can do:

Rhum.testSuite("check", () => {

  let xSocketMock

  class XapiSocketMock {
    check = check
    trades = () => {}
  }

  Rhum.testCase("should call trades once", async () => {
    xSocketMock = Rhum.mock(XapiSocketMock).create()
    await xSocketMock.check(data)
    Rhum.asserts.assertEquals( xSocketMock.calls.trades, 1 )
  })

})