-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Only using AttributionSource for compile sdk 31 and above #6884
Only using AttributionSource for compile sdk 31 and above #6884
Conversation
Signed-off-by: utzcoz <[email protected]>
Also testing it with example provided #6883 and Robolectric local SNAPSHOT. |
Looks like it breaks |
Looks like it uses real |
What about only loading methods and related classses supported by current running SDK? For example, if we only run tests with SDK 30, we will not load methods needs SDK 31 and above. But I remember there are some methods have been mixed with other purpose, for example normal API function invoked by other methods supported by other APIs. |
shadows/framework/src/main/java/org/robolectric/shadows/ShadowAppOpsManager.java
Show resolved
Hide resolved
ae7c996
to
35d4bab
Compare
Can you squash the last 2 commits so ShadowAppOpsManagerTest is not broken in a commit. |
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.
Thanks for doing this! Only one minor thing -- squash the last 2 commits.
FWIW, long-term, I think there could be better alternatives to @Implementation(minSdk = Build.VERSION_CODES.S)
protected int noteProxyOpNoThrow(
int op,
@WithType("android.content.AttributionSource") Object attributionSource,
String message,
boolean ignoredSkipProxyOperation) { ... } The advantage of this is that you don't have to worry about casting all of the types, only the problemetic ones. |
We have tried to use Object to replace AttributionSource as type of second input parameter for noteProxyOpNoThrow(int, AttributeSource, String, bool) to make sure ShadowAppOpsManager can be loaded and used when compile SDK is less than 31. But changed method's siganture doesn't match origin method signature, and it will be used as shadow method. To fix this problem, this CL uses looseSignatures for ShadowAppOpsManager, and chagnes all input parameters' type to Object to meet looseSignatures' requirement. Signed-off-by: utzcoz <[email protected]>
35d4bab
to
bb36cda
Compare
Signed-off-by: utzcoz <[email protected]>
I prefer this method more than current |
Done. |
I'll do a 4.7.3 release with this and #6880 |
This should now be fixed in 4.7.3 (may take a minute or two for that to appear on MavenCentral). |
Fixes #6883.