drashland / rhum

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

Mocking object with constructorArgs as non-primitive value crashes MockBuilder #119

Closed g-wozniak closed 2 years ago

g-wozniak commented 3 years ago

Summary

Following Rhum docs (Mock Constructor Arguments) it seems that providing array as a parameter to withConstructorArgs crashes MockBuilder.

Steps To Reproduce The Bug

When mocking

export class Logger {
  public constructor(types: string[] = []) {
    ...
  }
}

so that: const logger = Rhum.mock(Logger).withConstructorArgs('types', ['info']).create()

throws the following error:

TypeError: Cannot read property 'value' of undefined
    at https://deno.land/x/rhum@v1.1.9/src/mock_builder.ts:57:30
    at Array.forEach (<anonymous>)
    at MockBuilder.create (https://deno.land/x/rhum@v1.1.9/src/mock_builder.ts:55:37)
    at Object.testFn (file:///home/gwozniak/SIMPLY/simply-careers-mono/src/server/__tests__/services/logger.test.ts:18:79)

Rhum:

   // Attach all of the original's properties to the mock
    this.getAllProperties(original).forEach((property: string) => {
      const desc = Object.getOwnPropertyDescriptor(original, property);
      mock[property] = desc!.value; // Line marked <<<
    });

Expected Behavior

Rhum.mock() builds an instance of the mocked object.

crookse commented 3 years ago

@g-wozniak, thanks for the reproduction steps! also, sorry for the inconvenience. did you want to take a crack at fixing this or should i go ahead and look into this now?

g-wozniak commented 3 years ago

Thanks for the quick response. Please try if possible because it's getting late up here and got one more to report for SuperOak :D. Otherwise I'll spend some time tomorrow :)

crookse commented 3 years ago

haha no worries. i'll look into it. could you paste your logger test file here so i can further debug please?

g-wozniak commented 3 years ago

Simple as that:

    Rhum.testCase('→ displays info', () => {
      const logger = Rhum.mock(Logger).withConstructorArgs('types', ['info']).create()
      logger.info({ message: 'test' })
      Rhum.asserts.assertEquals(logger.print.calls, 1)
    })

and for simplification Logger has two methods: private print() and public info() - where info() calls print().

(Ideally, I'd like to mock console.info (didn't add stream support yet) but I guess Rhum supports only classes at the moment for mocking)

crookse commented 3 years ago

@g-wozniak, so I was able to get something to work. check out the following:

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

interface IMessage {
  message: string;
}

export class Logger {
  public constructor(types: string[] = []) {}
  public info(message: IMessage): void {
    this.print(message);
  }
  private print(message: IMessage): void {
  }
}

Rhum.testPlan("test.ts", () => {
  Rhum.testSuite("Logger", () => {
    Rhum.testCase('→ displays info', () => {
      const logger = Rhum.mock(Logger).withConstructorArgs(['info']).create()
      logger.info({ message: "test" })
      Rhum.asserts.assertEquals(logger.calls.info, 1)
    });
  });
});

Rhum.run();

It outputs the following:

test.ts
    Logger
        → displays info ... ok (3ms)

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out (3ms)

I did notice that your Rhum.mock call may have been incorrect. You had:

const logger = Rhum.mock(Logger).withConstructorArgs('types', ['info']).create()

when it should be

const logger = Rhum.mock(Logger).withConstructorArgs(['info']).create()

You have a great point btw. We do only support classes. We should look into mocking pretty much anything.

ebebbington commented 2 years ago

i'm actually unable to reproduce it on master and latest of deno, @g-wozniak can you verify using deno and rhum latest?

import { Rhum } from "./mod.ts"
export class Logger {
    public constructor(types: string[] = []) {

    }
  }

const logger = Rhum.mock(Logger).withConstructorArgs('types', ['info']).create()
ebebbington commented 2 years ago

going to close as it's been 3 months and had no reply