-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Handle special case when injecting AppWidgetManager with @SystemService #1380
Handle special case when injecting AppWidgetManager with @SystemService #1380
Conversation
|
Looks good. +1 for checking the sdk & current Plattform to decide how to inject. I think you verified the results, but is there a way to add a test? |
|
Thanks for checking the PRs! I wanted to add tests, but unfortunately there is no way to test this, because we do not have the API 21 classes at compile time. Actually we could just provide a dummy class, but we cannot add a new field to |
|
Hmm, i just thought about the compile time test (because this should be done there), but maybe we could do it in the functional test with the help of Robolectric. Actually it is a pain that we still depend on Android 4.x. I tend to create a solution like creating a script which installs the Android dependency to the local maven repo, as lots of Android projects the (this is needed for the compilation of Robolectric itself, for example). |
|
@dodgex i added a commit which tests one case of injection (when only the old method is available). It would be nice to do it in compile time test and check test the other cases, but we cannot do that currently. |
|
@WonderCsabo i assumed that it would be hard to get proper tests for that. lets hope no one breaks that accidently :D |
d7f9b76 to
36dbfe2
Compare
|
I'm okay with the code but the Travis build has failed. Could you fix this ? |
|
Thanks for checking! The Travis failure is not related to this PR, this passed. For some reason, Travis fails while unzipping the Gradle distribution. I am seeing these problems lately. I try to investigate. |
|
Maybe Travis is unstable now. I also see other errors, like build freezing while downloading Maven deps. |
|
OK. Feel free to merge this PR if it builds locally. |
36dbfe2 to
d8557a2
Compare
Handle special case when injecting AppWidgetManager with @SystemService
Fixes #1377.