Fixed problem with a mock resolving a mocked value with Promises

[jlkeesey]

The long description is in #191 so this is short.

This is a replacement for #192 which was incorrect.

This fixes a problem where returning a mocked object through thenResolve() on another mocked object never resolves.

[ianw11]

Also commented on #191 hoping to bring visibility to this fix!

[MOHACGCG]

@jlkeesey seems like this is approved thanks to @Elecweb. Can't wait for it to be merged! Thank you for the continued contribution on this great project!

[Napas]

Hi, when this is going to be merged in?

[alex-e-c]

Any update on this?

[alex-faber]

Hi, when will this be merged? Loads of ignored tests at the moment :( haha

[MOHACGCG]

@Napas @alex-e-c @alex-faber while waiting for the merge, as a workaround you can wrap the mocked object.
instead of when(foo()).thenResolve(instance(boo))
use:
alternative 1:
when(foo()).thenResolve({boo: instance(boo)})
alternative 2
when(foo()).thenResolve({ ... } as boo)

not the best but at least you can run the tests.

[alex-faber]

@MOHACGCG I'm pretty sure when(foo()).thenResolve(instance(boo)) is what I do anyway, and it never resolves

[MOHACGCG]

@MOHACGCG I'm pretty sure when(foo()).thenResolve(instance(boo)) is what I do anyway, and it never resolves

correct, what i meant is the alternatives do resolve, you can use them as a workaround for now.

[alex-faber]

@MOHACGCG sorry my bad, I misread! I'll give it a go, thanks!

[MOHACGCG]

@MOHACGCG sorry my bad, I misread! I'll give it a go, thanks!

no worries. good luck. I edited the comment to make it more clear!

[algono]

@Napas @alex-e-c @alex-faber @MOHACGCG The workaround from this comment #191 (comment) worked for me, with no need to alter the resolved value's structure. I hope it works for you as well.

[BenjaminPalko]

Any update on this?

[RageCage64]

Any reason this hasn't been merged in? The workaround is fine, but this would be a good fix to have in the library.

[upMKuhn]

Same question :D "Any reason this hasn't been merged in? The workaround is fine, but this would be a good fix to have in the library."

[donnyroufs]

ola what's up with this? time to merge this bad boy

[donnyroufs]

Same question :D "Any reason this hasn't been merged in? The workaround is fine, but this would be a good fix to have in the library."

Looks like we gotta fork and just publish it ourselves on npm ;')

[upMKuhn]

Hi @NagRock @mikeporterdev,

Thank you for this library! This issue is a real obstacle for new users and a nuisance for anyone using this library with nestjs. Could you please look into this github issue.

A summary: NestJs thinks mocks are a promise and waits indefinitely. To fix this when((mock as any).then).thenReturn(undefined) has to be set up everytime for nearly every mock.

Thank you!

[NagRock]

Hi, thanks for PR. But wouldn't this break anything if someone would like to mock then function?

[dariosn85]

@NagRock I think there are at least two sides of the problem:

1. Class has own method then

Let's have a look to following code as an example:

class Student {
    public test(): void {
        // do something
    }
}

In such case you concern make sense. Fix would break this logic in case if someone wants to mock then method of Student class.

2. Class does not have own then method, but then method is part of other function or comment

Now see following example:

class Person {
    number: number;
    
    public getName(): Promise<string> {
        return this.loadName()
            .then(name => {
                return `Student name: ${name}`;
            });
    }
    
    public async getPhoneNumber(): Promise<number> {
        
        // commented out code which also causes then() mock
        // await this.loadNumber()
        //     .then(number => this.number)
        //     .catch(error => this.number = undefined);
        
        return this.number;
    }
    
    private loadNumber(): Promise<number> {
        // do something
    }
    
    private loadName(): Promise<string> {
        // do something
    }
}

Because of nature how MockableFunctionsFinder works, it would find out .then() from .getName() and .then() from commented out code (within getPhoneNumber()) as a candidate for mocked method.

---

I would say that not all, but most cases what happens on the field is described by "2." (Person class). I think also we can all agree that in normal circumstances Person mock should not contain then method.

Is there any particular reason why ts-mockito class mocking is done by calling toString on the class and then searching for methods using regex? I would like to understand why base for mocking are not prototype methods or similar instead?

If the reason for regex is the way how method parameters are looked up or similar, then I would propose some kind of hybrid solution. We could combine class prototype/own methods (base for mocking) with output of regex (additional information). In addition ts-mockito could print out warning in case if class has own then method. That way we would cover both scenarios from above and at least give user clue (warning) what is happening.

[devtronic]

This workaround works for me:

// This import is not that nice but it's the only way to create mocks that works with promises
import {Mocker} from "ts-mockito/lib/Mock";

export function mockForPromise(clazz: unknown, instance?: unknown): any {
    return new Mocker(clazz, instance).getMock();
}

// use it
const mockApp = mockForPromise(Model<AppEntity>);
when(mockAppRepository.getAppByPackageId("test-package-id")).thenResolve(instance(mockApp));

[ffMathy]

@NagRock it is so painful for this to wait several years when a fix is right there in front of our eyes. If you've abandoned the project, can you at least appoint a contributor to take over?

[ffMathy]

Hi, thanks for PR. But wouldn't this break anything if someone would like to mock then function?

To respond to this: Yes. However, it's inevitable. There's no better way to do this when working with proxies.

I think the benefits outweigh the downsides. Especially because mocking "then" and "catch" is unlikely - way more unlikely than working with promises.