fix(android): Bump test dependencies to support jdk17 #775
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
krizzu
force-pushed
the
fix/jdk-17-support
branch
from
3373083 to
e921352
Compare
krizzu
force-pushed
the
fix/jdk-17-support
branch
from
e921352 to
cfb4249
Compare
5 tasks
krizzu
commented
Mar 23, 2022
Comment on lines
-132
to
+136
| "key5":"value5", | ||
| "key6":{ | ||
| "key7":"value7", | ||
| "key8":"value8" | ||
| } | ||
| }, | ||
| "key5":"value5" |
There was a problem hiding this comment.
For whatever reason, using includeAndroidResources option for unit test (required by latest robolectric) changes this order.
There was a problem hiding this comment.
That's a great question!
I tracked it down to changing the includeAndroidResources and different behavior of JSONObject.
With includeAndroidResources, the order of the keys in string is kept within JSON Object, while without it, it's different.
Note this:
With includeAndroidResources:
Without includeAndroidResources:
Note the data structure used with each other - includeAndroidResources uses LinkedHashMap, which keeps the order of data insertion, while HashMap does not. I don't know why different data structure is used with and without the flag, and I'd really want to know!
|
@renchap |
|
I changed my When I build my Android app, I am getting this error: The only change is using async storage from git, but I dont know if its caused by your branch or by the fact that I am pulling it from git. |
Can you add the following line to |
|
It now proceeds but with another error: FYI, if I add this to my |
|
Seems like Gradle issue. |
|
Good catch, it now builds fine with your branch (Gradle 7.3, |
|
Coolio, we it seems like a fix to support JDK 17. Thanks for reporting and testing 🙏 |
tido64
reviewed
Mar 23, 2022
| defaultConfig { | ||
| minSdkVersion safeExtGet('minSdkVersion', 21) | ||
| targetSdkVersion safeExtGet('targetSdkVersion', 29) | ||
| targetSdkVersion safeExtGet('targetSdkVersion', 31) |
There was a problem hiding this comment.
Have you tested that this works? We had to revert back to 29 due to a crash on startup: microsoft/react-native-test-app@f873225
There was a problem hiding this comment.
That's interesting - why you would have a crash on startup due to targetSdk? It's used as compile time and by IDE (to give you latest warnings/deprecations etc)
There was a problem hiding this comment.
Ah, it seems it was a bug in okHttp parsing android version 😄
https://stackoverflow.com/a/64235713
There was a problem hiding this comment.
| returnDefaultValues = true | ||
| includeAndroidResources = true | ||
| } | ||
| } |
There was a problem hiding this comment.
Don't we need to keep this as long as minSdkVersion is below 24?
There was a problem hiding this comment.
Starting with AGP 4.2, it's 1.8 by default
https://developer.android.com/studio/releases/gradle-plugin#java-8-default
Comment on lines
-132
to
+136
| "key5":"value5", | ||
| "key6":{ | ||
| "key7":"value7", | ||
| "key8":"value8" | ||
| } | ||
| }, | ||
| "key5":"value5" |
tido64
approved these changes
Mar 23, 2022
|
🎉 This PR is included in version 1.17.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Bumping dependencies for tests, to support jdk17.
Test Plan
Green CI.