NagRock / ts-mockito

Mocking library for TypeScript
MIT License
975 stars 93 forks source link

Mocking abstract classes #25

Open ssynix opened 7 years ago

ssynix commented 7 years ago

I see that this is not supported yet:

'Argument of type 'typeof AbstractClass' is not assignable to parameter of type 'new (...args: any[]) => AbstractClass'. Cannot assign an abstract constructor type to a non-abstract constructor type.'

Can this be supported in TypeScript or not yet possible?

Thanks!

michalstocki commented 7 years ago

Mocking abstract classes is not a trivial task, because abstract methods do not exist in JavaScript. We can't simply iterate over the object properties to create the correct mock object.

abstract class AbstractClass {
  public abstract getValue():number;

  public getOtherValue():string {
    return '';
  }
}

produces:

    var AbstractClass = (function () {
        function AbstractClass() {
        }
        AbstractClass.prototype.getOtherValue = function () {
            return '';
        };
        return AbstractClass;
    }());

This could potentially be implemented in the way similar to mocking of interfaces (using Proxy object). Maybe using the same technique. However the feature is still not supported. @NagRock could you confirm whether the feature is planned in the nearest future?

const abstractMock:AbstractClass = mockType<AbstractClass>();
michalstocki commented 7 years ago

Nevertheless, the simple workaround for lack of this feature is using any extension of the AbstractClass to mock;

// Exntension.ts
export class Extension extends AbstractClass {
  public getValue():number {
    return 1;
  }
}

// test
const abstractMock:AbstractClass = mock(Extension);

Note, that abstractMock is in type of AbstractClass to not allow calling of extra methods that Extenstion could have.

ssynix commented 7 years ago

Thanks for the explanation and the quick response! I'm using the workaround right now. One thing I don't like about it is that any changes in the number or signature of abstract methods has to be replicated in the Extension class.

michalstocki commented 7 years ago

😄 But it's the essential value of an abstract class: it enforces consistency of it's implementations. I think you should not create a special Extension class only for the mocking purpose. You can use one of the extensions already existing in your code.

NagRock commented 7 years ago

@michalstocki mocking interfaces is planned but rather not in the coming weeks. Also I will need some help with this feature from you.

NagRock commented 7 years ago

Thanks to @dyong0 we can now mock abstract and generic types in version 2.1.0 (https://github.com/NagRock/ts-mockito/releases/tag/v2.1.0)! I will close this issue and open new for interfaces mocking.

NagRock commented 7 years ago

@ssynix please give feedback is it works as expected.

dyong0 commented 7 years ago

@NagRock I found another issue on mocking generics. Mocking a generic class or interface loses members with its generic types. For example, mongoose.Model is an generic interface

interface Model<T extends Document> extends NodeJS.EventEmitter, ModelProperties { ... }

which has

create(docs: any[], callback?: (err: any, res: T[]) => void): Promise<T[]>;
where(path: string, val?: Object): Query<any>;

When you mock it:

let mockedModel = mock(mongoose.Model);
let model = instance(mockedModel);

it loses some parts of its body: image yet keeps members without its generic types: image

Can you please add a note for this issue on mocking generics? README would be nice to place it.

dyong0 commented 7 years ago

Plus, when you console.log() mocked a generic. It displays null, but actually it's not.

let mockedModel = mock(mongoose.Model);
let model = instance(mockedModel);

console.log(model); // null
model === null; // false
NagRock commented 7 years ago

@dyong0 I will try to fix it instead of adding info in readme. Thanks for reporting this!

NagRock commented 7 years ago

@dyong0 you trying to mock interface (not abstract class) that is not supported yet.

dyong0 commented 7 years ago

@NagRock right, it doesn't have tests for mocking interfaces. but looks working fine for interfaces too. btw the issue i reported was about mocking generics. i think the same problem happens on generic classes as well as abstract ones.

NagRock commented 7 years ago

@dyong0 could you please provide a sample repository with this? It would be much simpler for me to fix this with project that reproduces issue.

mpiroc commented 7 years ago

When mocking abstract classes, abstract methods are not mocked (version 2.1.1). I assume that this is the same problem that prevents mocking interfaces, but at minimum the docs should be updated to prevent confusion.

abstract class Foo {
  abstract foo(): void;
  bar(): void {}
}

...

const mock = mock(Foo)
mock.foo // undefined
mock.bar // function
assafaloni commented 6 years ago

I tried to run @mpiroc example with v2.2.5 and got the same output (foo is undefined), I did include es6 for tsconfig lib param.

I dont understand - this should be supported now? if so what am i doing wrong? Thanks!

NagRock commented 6 years ago

Hi, thanks for reporting. I will check it.

julianosam commented 6 years ago

Hey guys. Was this issue solved? I can see the PR that adds support for mocking abstract classes was merged, but I'm still getting the error mentioned by @mpiroc. I'm using version 2.2.9. Thanks

woodcockjosh commented 6 years ago

When we gonna be able to mock interfaces? It's like really I have to mock an implementation? That's totally broken and defeats the whole point of mocking.

pbadenski commented 6 years ago

@woodcockjosh it doesn't seem to be that obvious how this can be implemented easily.. I use this workaround:

interface LoadBalancer {
  calculate: (...) => Promise<any>;
}

const loadBalancerMock =
  mock.mock<LoadBalancer>({ prototype: { calculate: (): any => undefined } } as any);
woodcockjosh commented 6 years ago

Fine, I'll do a pull request with a solution. That's obviously not the solution.

jimmythompson commented 6 years ago

@woodcockjosh I'm not sure how you intended that response to come across, but it sounded really douchey. 😕

woodcockjosh commented 6 years ago

Sorry, you know developers write before they think sometimes :-/. I meant I think there's a better solution and will try to contribute with more than a complaint.

On Fri, Apr 13, 2018, 2:21 PM Jimmy Thompson notifications@github.com wrote:

@woodcockjosh https://github.com/woodcockjosh I'm not sure how you intended that response to come across, but it sounded really douchey. 😕

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NagRock/ts-mockito/issues/25#issuecomment-381237162, or mute the thread https://github.com/notifications/unsubscribe-auth/ADcSSJvL2AKmaNcx-caHbgAxrLl9zuVgks5toPrIgaJpZM4NlTWe .

joaovieira commented 6 years ago

+1 for mocking interfaces! 😩

Edit: Just noticed what I wanted is actually pretty easy:

interface ITest {
  one: string;
  two(): number;
}

const service = (dep: ITest) => {
  dep.two();
};

const testMock: Partial<ITest> = {
  two: jest.fn(() => 1),
};

const subject = service(testMock as ITest);

Peaked into https://github.com/jjherscheid/ts-mocks/blob/master/src/mocks/mock.ts