Inflate menu before calling method injection #1962
Conversation
|
I just realized that it should be enough to move @WonderCsabo what do you think should we prefer? |
|
@dodgex i think this fix is better, because this way we do not rely on the order of handlers. |
WonderCsabo
left a comment
There was a problem hiding this comment.
@dodgex can you add a test case?
|
Not sure how to test this as it seems that robolectrict does not support menu inflation? |
|
@dodgex maybe you could create a spy of the TestActivity activity = Robolectric.buildActivity ...
TestActivity activitySpy = Mockito.spy(activity);
Mockito.when(activity.getMenuInflater()).thenReturn(...); // create a mock menu inflater, and stub its inflate() method which alters the `Menu` object in some way you can assert on it
activity.onCreateOptionsMenu(menu);
assertThat(menu).is... |
|
added a test similar to what you suggested. |
|
Yay.... to get the spy working i needed the generated class to be non-final...
but there is a test that checks if a generated class is final... |
|
Then we can just override getMenuInflater in the test activity which
returns the mock, and check if it was called correctly with some flag.
…On Mar 17, 2017 09:51, "Kay-Uwe Janssen" ***@***.***> wrote:
Yay.... to get the spy working i needed the generated class to be
non-final...
AbstractActivityTest.activityShouldBeFinal:33 expected:<[tru]e> but
was:<[fals]e>
but there is a test that checks if a generated class is final...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1962 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAdU5RTm90zRZyYJP9SBRukHdeoIUVBnks5rmkl2gaJpZM4MTXVb>
.
|
|
Build should go green now. |
WonderCsabo
left a comment
There was a problem hiding this comment.
Please do not modify the existing tests, add new one(s). A test should only assert one thing.
| final InjectMenuActivity.MockMenuInflater menuInflater = Mockito.mock(InjectMenuActivity.MockMenuInflater.class); | ||
| doAnswer(new Answer<Void>() { | ||
| public Void answer(InvocationOnMock invocation) { | ||
| menuInflater.menuInflated = true; |
There was a problem hiding this comment.
OK, i realised you can just you Mockito verification, since you already using mocking.
Mockito.verify(menuInflater).inflate(R.menu.my_menu, menu);This way you can get rid of the inner class and the boolean field as well.
Another thing: most of the times, we static import methods like mock and verify.
There was a problem hiding this comment.
We then would need to call verify() in the InjectMenuActivity. after the call to injectMenuActivity.onCreateOptionsMenu(menu); the inflater is awlays called. the interesting part is that it has to be called BEFORE a method with @InjectMenu is called.
There was a problem hiding this comment.
So do you have another Idea to change this or should we keep it?
|
I reverted the changes to the existing tests and added a new test case for this |
| assertThat(injectMenuActivity.menuIsInflated).isFalse(); | ||
| injectMenuActivity.onCreateOptionsMenu(menu); | ||
| assertThat(injectMenuActivity.menuIsInflated).isTrue(); | ||
|
|
There was a problem hiding this comment.
I think this assert is not needed, it is already checked in other tests.
Fix issue reported in #1961