Skip to content
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

Added support to provide Spy dependencies #82

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

krazykira
Copy link

@krazykira krazykira commented Aug 16, 2018

  1. Added ability to return all provided dependencies inside the module as Spies which would help with partial mocking. Fixes Support for injecting mockito Spy objects without overriding modules #81

Note: Please let me know if you want me to update the Readme.md or if you want me to

@codecov-io
Copy link

codecov-io commented Aug 16, 2018

Codecov Report

Merging #82 into master will decrease coverage by 0.61%.
The diff coverage is 60%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #82      +/-   ##
============================================
- Coverage     85.48%   84.86%   -0.62%     
  Complexity      187      187              
============================================
  Files            12       12              
  Lines           565      575      +10     
  Branches         97       98       +1     
============================================
+ Hits            483      488       +5     
- Misses           57       61       +4     
- Partials         25       26       +1
Impacted Files Coverage Δ Complexity Δ
...a/it/cosenonjaviste/daggermock/DaggerMockRule.java 88.05% <25%> (-2.02%) 32 <0> (ø)
.../it/cosenonjaviste/daggermock/ModuleOverrider.java 78.46% <72.72%> (-0.85%) 17 <1> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1cee8a0...c90c372. Read the comment docs.

@fabioCollini
Copy link
Owner

I was thinking about something different, a method like this one:

public <S> DaggerMockRule<C> decorate(Class<S> originalClass, ObjectDecorator<S> decorator)

This would be more generic, instead of just creating a spy it allows to create a new object that uses the original one. ObjectDecorator could be an interface with a single method with the original object as parameter and the new object as return parameter.
What do you think?

@krazykira
Copy link
Author

@fabioCollini i think the whole point was that i didn't want to create and provide the spy object otherwise i could have used

  public <S> DaggerMockRule<C> provides(Class<S> originalClass, Provider<S> provider) {
        overriddenObjectsMap.putProvider(originalClass, provider);
        return this;
    }

e.g:
Most of the time one of our repository method is responsible for fetching data (i.e getproducts()) and caching it. All the other methods are manipulating with the same cached data. This is where we could use the spy object. We could just mock the getproducts() method and rest of the repository methods will work correctly.

If there is any other way to providing spy without creating the underline dependencies then i would be open to it.

@fabioCollini
Copy link
Owner

Sorry, I don't get your point but maybe my explanation was not clear. I was saying that a decorate object can be useful instead of adding a boolean to create a spy for each object. The final result is the same, it's just more configurable. To obtain the same result you can invoke it in this way:

decorate<MyRepository> { originalObjectCreatedByDagger -> 
     spy(originalObjectCreatedByDagger)
}

To obtain something similar for the dependent object another decorate invocation can be used.

@krazykira
Copy link
Author

@fabioCollini yes that would be even better. But i don't have much clue on how to achieve that 😕

@fabioCollini
Copy link
Owner

Hi, I have implemented it in this commit a261d93ea5, the modification is not too big and I have tried it in some tests. Can you try to use it (just use the commit as DaggerMock version) and give me a feedback?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants