Create native proxy object to check provider on each call and mediate… #2532
Conversation
… between desired return value and default behavior)
Coverage ReportAffected SDKs
Test Logs
NotesHTML coverage reports can be produced locally with Head commit (461cf834) is created by Prow via merging commits: 2cc1cc4 6aabb02. |
Binary Size ReportAffected SDKs
Test Logs
NotesHead commit (461cf834) is created by Prow via merging commits: 2cc1cc4 6aabb02. |
| if (provider.get() != null) { | ||
| return provider.get().hasCrashDataForSession(sessionId); | ||
| } |
There was a problem hiding this comment.
I recommend caching the result of get() instead of calling into it twice. This will make code more readable and more conventional as per Injecting Providers where in more traditional DI setups each call to get() could return a newly created instance if the component is not a singleton.
While all Firebase components are singletons, it could be beneficial to be consistent with common conventions for future readers of this code(including ourselves).
| if (provider.get() != null) { | |
| return provider.get().hasCrashDataForSession(sessionId); | |
| } | |
| Foo native = provider.get(); | |
| if (native != null) { | |
| return native.hasCrashDataForSession(sessionId); | |
| } |
Here and throughout
| @Mock private CrashlyticsNativeComponent component; | ||
|
|
||
| @Before | ||
| public void setUp() throws Exception { | ||
| MockitoAnnotations.initMocks(this); | ||
| } |
There was a problem hiding this comment.
nit: for this simple case you can just use the more concise:
| @Mock private CrashlyticsNativeComponent component; | |
| @Before | |
| public void setUp() throws Exception { | |
| MockitoAnnotations.initMocks(this); | |
| } | |
| private final CrashlyticsNativeComponent component = mock(CrashlyticsNativeComponent.class); |
| import org.mockito.Mockito; | ||
| import org.mockito.MockitoAnnotations; | ||
|
|
||
| public class ProviderProxyNative { |
There was a problem hiding this comment.
Please add a @RunWith(AndroidJUnit4.class)
|
@davidmotson: The following test failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
… between desired return value and default behavior)