-
Notifications
You must be signed in to change notification settings - Fork 93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding support for mocking interfaces #76
base: interfaces-mocking
Are you sure you want to change the base?
Adding support for mocking interfaces #76
Conversation
b9a7bb7
to
0887c44
Compare
Codecov Report
@@ Coverage Diff @@
## master #76 +/- ##
==========================================
- Coverage 94.9% 93.56% -1.35%
==========================================
Files 34 34
Lines 609 653 +44
Branches 70 81 +11
==========================================
+ Hits 578 611 +33
- Misses 22 28 +6
- Partials 9 14 +5
Continue to review full report at Codecov.
|
0887c44
to
30c2ee8
Compare
😮 @johanblumenberg this looks great! Interfaces mocking feature has been long awaited. I only got to think about |
src/ts-mockito.ts
Outdated
const mockedValue = new Mocker(Empty, true).getMock(); | ||
|
||
if (typeof Proxy === "undefined") { | ||
return mockedValue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we should throw Error here to get know user that interface mocking is not available without Proxy support. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are right
e489ba0
to
e24f415
Compare
@NagRock I added an exception if calling |
This only supports stubbing methods, am I right? I also tried something like this some time ago but failed to differentiate between properties and method calls. |
@Markus-Ende Yes, only stubbing methods. That's why I made a separate function, But it's still quite useful to be able to automatically mock interfaces, even though you cannot mock properties on interfaces. |
e24f415
to
c259c07
Compare
c259c07
to
8431eeb
Compare
Hi, what is blocking this PR? Is there anything I can help with? |
Looks useful. But I think there should be a type parameter for imock(). so the return types can be defined (just like when using mock()).
EDIT: Sorry, I didn't see there already is a type parameter. |
8431eeb
to
88757bb
Compare
@NagRock Is there anything blocking this PR? |
88757bb
to
b023fac
Compare
any update on this PR? will it be merged soon? |
+1, when will it merged ? |
+2 on merging @NagRock any ideas when this will be merged and available? |
@johanblumenberg maybe add usage examples to the readme as well? |
b023fac
to
4c6dfab
Compare
Fixed a problem when retuning mock instances in
The problem is that the A mock object will intersect all unknown properties and return a stub function, so it will look like a promise to the promise library. (test "resolves with given mock value" in stubbing.method.spec.ts) |
Added support for mocking both properties or methods on interfaces. This will make all properties on the object to be stubbed as properties:
This will make all properties on the object to be stubbed as methods:
For classes, the same behavior is kept. It will look at the class to decide which properties are methods and which are properties. The new policy applies to unknown properties and all properties on interfaces. |
Added support for mocking interfaces containing both properties and methods.
This must be done before the mock instance is used. If not, the default property policy is used to decide if the property is a property or a method. |
4c6dfab
to
3cf15e1
Compare
Is there an alternative way to mock an interface like this at this point? (without casting to any) |
ping |
Is there any updates on this, this is a blocker for us to use ts-mockito, specially because we have to implement interface to mock them. Can you please merge this soon, I've waiting for the this feature for so long, thank you for this awesome ts mocking lib. |
@kouros51, @DonJayamanne, @cjanietz, @drenner-netspend, @kwolfy, @perransw, @Markus-Ende I published a forked package, that contains the possibility to mock interfaces: https://www.npmjs.com/package/@johanblumenberg/ts-mockito It has support for mocking methods and properties on interfaces. |
Pushed another commit with updated README, describing |
@johanblumenberg thanks for that. This was very helpful. |
I tried your build, and I was having issues with mocking properties defined in interfaces. For example:
I rewrote this test from memory, I don't have the machine that ran this test, but you get the general gist of it. The explicit error was something like "could not reference 'add' property of undefined". I can't remember if the error occurs from the Has anyone else seen this? |
Hm... never mind. I tried to reproduce it locally with a much smaller sample, and it worked fine. The below code passes just fine:
|
@NagRock Is this repository still maintained? This review was opened over a year ago, and despite being incredibly useful, hasn't been merged into master. Essentially, I work on a project that heavily requires mocks, and we opted to go with |
@aserra54 I published my fork as a package on npm: https://www.npmjs.com/package/@johanblumenberg/ts-mockito Feel free to use it, or to suggest other PR's that I should add to it. |
Unfortunately this PR doesn't support So this will break some functionalities or we will have to remove |
There is no good solution for interface mocking (or maybe there is but I dont have it). One will break current API, and this one will break |
Thanks for closing this PR, at least we have a resolution. |
@NagRock could you give an example where this PR is breaking getters / setters? |
Agreed, would be good to provide technical insight for someone wanting to doing submit another PR to address the concerns, now it in the future.... |
@johanblumenberg Sorry I will check it once again and add specs for |
Should we reopen this PR? |
@johanblumenberg can you explain what is |
The purpose of When creating a mock object from an interface, a So when the proxy should return a property, it has to know if it is supposed to return a property or a function mock. One way to tell the proxy object what to do is to set up an expectation for that call. For example: when(mock.foo).thenReturn(5); // Now foo is a property
when(mock.foo()).thenReturn(5); // Now foo is a method The code will be checked in compile time, so only the correct signature is possible. One of the above will give a compile error. The problem is when there are properties that don't have any expectations set. Now the proxy doesn't know how to handle that. Typically it would either handle it as a property, returning The The first two are really only a workaround because all information about interfaces is lost in runtime. If there is any other way to fix it, that would be better. |
when(mockedFoo.bar).thenReturn('five'); | ||
``` | ||
|
||
For interface mocks, you can set the defauklt behviour for mocked properties that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For interface mocks, you can set the defauklt behviour for mocked properties that | |
For interface mocks, you can set the default behavior for mocked properties that |
Thanks for maintaining this awesome library. |
I integrated Johan Blumenberg's release into our test setup, and while it generally works great, we've had issues where Unfortunately, the issue reproduces consistently if I run our entire test suite, but if I take the same interface (which is actually a standalone interface) and put it in a small test project to try and reproduce the issue, it works fine. I think the philosophy behind the idea of |
@aserra54 Would be good to provide some steps to repro the issue. |
New version with support for interfaces released: https://github.com/NagRock/ts-mockito/releases/tag/v2.4.0 Great thanks @johanblumenberg for this PR that pushed me forward with final implementation. And sorry that this took so long time. If something works wrong, please open an issue. |
Should this be closed now, @NagRock ? |
This commit adds support for mocking interfaces, like this: