From 1181ca29caa9b8824eb625a775d69b9d8ddf8b0c Mon Sep 17 00:00:00 2001 From: Ben Henning Date: Tue, 28 Nov 2023 17:52:50 -0800 Subject: [PATCH] Fix #5137: Upgrade builds to target SDK 33 (#5222) ## Explanation Fixes #5137 Partially mitigates #5233 This updates all build & target SDKs for Gradle & Bazel builds to now target SDK 33, per the new Play Store mandate (see https://support.google.com/googleplay/android-developer/answer/11926878?hl=en). The analysis of SDK 33 features, potential issues, and potential mitigations are described in #5137. It was determined that there are no obvious code changes needed beyond the minimum to target SDK 33. We'll be relying on some platform-level regression testing plus the Play Console's pre-submit app analysis report for finalizing the go/no-go decision for SDK 33 support. Testing consisted of manually playing through the app and seeing if there were any issues using emulators both with SDK 33 and lower versions. #5137 documents the issues found and their ultimate causes. Since no functionality changes were needed for SDK 33, no tests needed updating. Separately, tests are still running on SDK 30 due to being stuck (see #4748). One issue unrelated to SDK 33 was observed: when playing through the prototype lesson, I once again brought the app into a broken state wherein I couldn't leave the lesson via navigation. We've hit this issue a few times, but still don't know the root cause. #5233 was filed and a mitigation was introduced in this PR (+ a test for it): if the exploration player observes a failure when trying to stop the player session (for whatever reason), it still proceeds with its "end of activity" flow (e.g. finishing the activity). This provides at least an escape hatch for users who somehow hit this state. Note that this isn't a proper fix for #5233 given the lack of a known root cause, so this is only considered a mitigation. The new test was verified as failing if the mitigation is omitted: ![image](https://github.com/oppia/oppia-android/assets/12983742/10ddbf85-332c-4469-8efa-483a967170f9) Note that ``ExplorationActivityTest``'s setup was tweaked to change the parent screen since this affects back navigation routing. I opted to using lessons tab as the parent screen since it's the most common route for users to hit, so it makes sense for our tests to default to that if they don't specifically need to use a different route (and none of the existing tests seemed to have an issue with this change). ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only N/A -- This isn't changing any UIs directly, and per the analysis in #5137 it doesn't seem that there are any relevant new UIs enabled in SDK 33 that would affect Oppia. --- .../action.yml | 4 +-- BUILD.bazel | 4 +-- WORKSPACE | 2 +- app/build.gradle | 4 +-- app/src/main/AppAndroidManifest.xml | 2 +- app/src/main/DatabindingAdaptersManifest.xml | 2 +- app/src/main/DatabindingResourcesManifest.xml | 2 +- app/src/main/RecyclerviewAdaptersManifest.xml | 2 +- app/src/main/ViewModelManifest.xml | 2 +- app/src/main/ViewModelsManifest.xml | 2 +- app/src/main/ViewsManifest.xml | 2 +- .../ExplorationActivityPresenter.kt | 34 +++++++++--------- .../exploration/ExplorationActivityTest.kt | 36 ++++++++++++++++++- app/src/test/resources/robolectric.properties | 2 +- build_flavors.bzl | 14 ++++---- .../oppia/android/config/AndroidManifest.xml | 2 +- data/build.gradle | 4 +-- .../src/test/resources/robolectric.properties | 2 +- domain/build.gradle | 4 +-- domain/src/main/AndroidManifest.xml | 2 +- .../src/test/resources/robolectric.properties | 2 +- instrumentation/BUILD.bazel | 2 +- testing/build.gradle | 4 +-- .../src/test/resources/robolectric.properties | 2 +- utility/build.gradle | 4 +-- utility/src/main/AndroidManifest.xml | 2 +- .../src/test/resources/robolectric.properties | 2 +- 27 files changed, 90 insertions(+), 56 deletions(-) diff --git a/.github/actions/set-up-android-bazel-build-environment/action.yml b/.github/actions/set-up-android-bazel-build-environment/action.yml index 6b3f5cf1155..6afe9f398a8 100644 --- a/.github/actions/set-up-android-bazel-build-environment/action.yml +++ b/.github/actions/set-up-android-bazel-build-environment/action.yml @@ -72,9 +72,9 @@ runs: $ANDROID_HOME/cmdline-tools/tools/bin/sdkmanager --install "platform-tools" shell: bash - - name: Install SDK 31 + - name: Install SDK 33 run: | - $ANDROID_HOME/cmdline-tools/tools/bin/sdkmanager --install "platforms;android-31" + $ANDROID_HOME/cmdline-tools/tools/bin/sdkmanager --install "platforms;android-33" shell: bash - name: Install build tools 29.0.2 diff --git a/BUILD.bazel b/BUILD.bazel index 28daa546184..cbd6507132e 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -118,14 +118,14 @@ package_group( "flavor": "oppia", "min_sdk_version": 21, "multidex": "native", - "target_sdk_version": 31, + "target_sdk_version": 33, }, { "flavor": "oppia_kitkat", "main_dex_list": "//:config/kitkat_main_dex_class_list.txt", "min_sdk_version": 19, "multidex": "manual_main_dex", - "target_sdk_version": 31, + "target_sdk_version": 33, }, ] ] diff --git a/WORKSPACE b/WORKSPACE index 231078d0d6e..d1d218526cf 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -11,7 +11,7 @@ load("//third_party:versions.bzl", "HTTP_DEPENDENCY_VERSIONS", "get_maven_depend # TODO(#1542): Sync Android SDK version with the manifest. android_sdk_repository( name = "androidsdk", - api_level = 31, + api_level = 33, build_tools_version = "29.0.2", ) diff --git a/app/build.gradle b/app/build.gradle index 72dbb1d3cff..6de6e2c4757 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -6,12 +6,12 @@ apply plugin: 'kotlin-android-extensions' apply plugin: 'kotlin-kapt' android { - compileSdkVersion 31 + compileSdkVersion 33 buildToolsVersion "29.0.2" defaultConfig { applicationId "org.oppia.android" minSdkVersion 19 - targetSdkVersion 31 + targetSdkVersion 33 versionCode 1 versionName "1.0" multiDexEnabled true diff --git a/app/src/main/AppAndroidManifest.xml b/app/src/main/AppAndroidManifest.xml index 9a5789e1872..211884a0158 100644 --- a/app/src/main/AppAndroidManifest.xml +++ b/app/src/main/AppAndroidManifest.xml @@ -1,5 +1,5 @@ + android:targetSdkVersion="33" /> diff --git a/app/src/main/DatabindingAdaptersManifest.xml b/app/src/main/DatabindingAdaptersManifest.xml index 0974b6b3aa6..d3e60f6d5f4 100644 --- a/app/src/main/DatabindingAdaptersManifest.xml +++ b/app/src/main/DatabindingAdaptersManifest.xml @@ -1,5 +1,5 @@ + android:targetSdkVersion="33" /> diff --git a/app/src/main/DatabindingResourcesManifest.xml b/app/src/main/DatabindingResourcesManifest.xml index 0c3dd8fa35a..c9f98dbf248 100644 --- a/app/src/main/DatabindingResourcesManifest.xml +++ b/app/src/main/DatabindingResourcesManifest.xml @@ -1,5 +1,5 @@ + android:targetSdkVersion="33" /> diff --git a/app/src/main/RecyclerviewAdaptersManifest.xml b/app/src/main/RecyclerviewAdaptersManifest.xml index f2a3273e765..6585b5ea24c 100644 --- a/app/src/main/RecyclerviewAdaptersManifest.xml +++ b/app/src/main/RecyclerviewAdaptersManifest.xml @@ -1,5 +1,5 @@ + android:targetSdkVersion="33" /> diff --git a/app/src/main/ViewModelManifest.xml b/app/src/main/ViewModelManifest.xml index 07603e895d8..c6c3e62e26b 100644 --- a/app/src/main/ViewModelManifest.xml +++ b/app/src/main/ViewModelManifest.xml @@ -3,5 +3,5 @@ + android:targetSdkVersion="33" /> diff --git a/app/src/main/ViewModelsManifest.xml b/app/src/main/ViewModelsManifest.xml index 84693784fd8..e210893ecd0 100644 --- a/app/src/main/ViewModelsManifest.xml +++ b/app/src/main/ViewModelsManifest.xml @@ -3,5 +3,5 @@ + android:targetSdkVersion="33" /> diff --git a/app/src/main/ViewsManifest.xml b/app/src/main/ViewsManifest.xml index 340e35afe29..b77df4edb19 100644 --- a/app/src/main/ViewsManifest.xml +++ b/app/src/main/ViewsManifest.xml @@ -3,5 +3,5 @@ + android:targetSdkVersion="33" /> diff --git a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt index 3efadb513de..cd4a33b2d24 100644 --- a/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt +++ b/app/src/main/java/org/oppia/android/app/player/exploration/ExplorationActivityPresenter.kt @@ -286,25 +286,25 @@ class ExplorationActivityPresenter @Inject constructor( fun stopExploration(isCompletion: Boolean) { fontScaleConfigurationUtil.adjustFontScale(activity, ReadingTextSize.MEDIUM_TEXT_SIZE) - explorationDataController.stopPlayingExploration(isCompletion).toLiveData() - .observe( - activity, - { - when (it) { - is AsyncResult.Pending -> oppiaLogger.d("ExplorationActivity", "Stopping exploration") - is AsyncResult.Failure -> - oppiaLogger.e("ExplorationActivity", "Failed to stop exploration", it.error) - is AsyncResult.Success -> { - oppiaLogger.d("ExplorationActivity", "Successfully stopped exploration") - if (isCompletion) { - maybeShowSurveyDialog(profileId, topicId) - } else { - backPressActivitySelector() - } - } + explorationDataController.stopPlayingExploration(isCompletion).toLiveData().observe(activity) { + when (it) { + is AsyncResult.Pending -> + oppiaLogger.d("ExplorationActivity", "Stopping exploration") + is AsyncResult.Failure -> { + oppiaLogger.e("ExplorationActivity", "Failed to stop exploration", it.error) + // Allow the user to always exit if they get into a broken state. + backPressActivitySelector() + } + is AsyncResult.Success -> { + oppiaLogger.d("ExplorationActivity", "Successfully stopped exploration") + if (isCompletion) { + maybeShowSurveyDialog(profileId, topicId) + } else { + backPressActivitySelector() } } - ) + } + } } fun onKeyboardAction(actionCode: Int) { diff --git a/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt b/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt index e1cce42d8f3..9e33f7d9948 100644 --- a/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt +++ b/app/src/sharedTest/java/org/oppia/android/app/player/exploration/ExplorationActivityTest.kt @@ -1897,6 +1897,38 @@ class ExplorationActivityTest { explorationDataController.stopPlayingExploration(isCompletion = false) } + @Test + fun testExpActivity_pressBack_whenProgressControllerBroken_stillEndsActivity() { + setUpAudioForFractionLesson() + explorationActivityTestRule.launchActivity( + createExplorationActivityIntent( + internalProfileId, + FRACTIONS_TOPIC_ID, + FRACTIONS_STORY_ID_0, + FRACTIONS_EXPLORATION_ID_0, + shouldSavePartialProgress = true + ) + ) + explorationDataController.startPlayingNewExploration( + internalProfileId, + FRACTIONS_TOPIC_ID, + FRACTIONS_STORY_ID_0, + FRACTIONS_EXPLORATION_ID_0 + ) + testCoroutineDispatchers.runCurrent() + + // Simulate cases when the data controller enters a bad state by pre-finishing the exploration + // prior to trying to exit. While this seems impossible, it's been observed in real situations + // without a known cause. If it does happen, the user needs to have an escape hatch to actually + // leave. See #5233. + explorationDataController.stopPlayingExploration(isCompletion = false) + testCoroutineDispatchers.runCurrent() + pressBack() + testCoroutineDispatchers.runCurrent() + + assertThat(explorationActivityTestRule.activity.isFinishing).isTrue() + } + @Test @RunOn(TestPlatform.ROBOLECTRIC) // TODO(#3858): Enable for Espresso. fun testExpActivity_englishContentLang_contentIsInEnglish() { @@ -2303,13 +2335,15 @@ class ExplorationActivityTest { explorationId: String, shouldSavePartialProgress: Boolean ): Intent { + // Note that the parent screen is defaulted to TOPIC_SCREEN_LESSONS_TAB since that's the most + // typical route to playing an exploration. return ExplorationActivity.createExplorationActivityIntent( ApplicationProvider.getApplicationContext(), ProfileId.newBuilder().apply { internalId = internalProfileId }.build(), topicId, storyId, explorationId, - parentScreen = ExplorationActivityParams.ParentScreen.PARENT_SCREEN_UNSPECIFIED, + parentScreen = ExplorationActivityParams.ParentScreen.TOPIC_SCREEN_LESSONS_TAB, shouldSavePartialProgress ) } diff --git a/app/src/test/resources/robolectric.properties b/app/src/test/resources/robolectric.properties index 563d60ad14b..1aafcf8ea7d 100644 --- a/app/src/test/resources/robolectric.properties +++ b/app/src/test/resources/robolectric.properties @@ -1,3 +1,3 @@ # app/src/test/resources/robolectric.properties -# TODO(#4748): Remove the need for this file after upgrading Robolectric tests to API 31 +# TODO(#4748): Remove the need for this file after upgrading Robolectric tests to API 33 sdk=30 diff --git a/build_flavors.bzl b/build_flavors.bzl index 9052490bf85..0440b171a55 100644 --- a/build_flavors.bzl +++ b/build_flavors.bzl @@ -46,7 +46,7 @@ _FLAVOR_METADATA = { "dev": { "manifest": "//app:src/main/AndroidManifest.xml", "min_sdk_version": 21, - "target_sdk_version": 31, + "target_sdk_version": 33, "multidex": "native", "proguard_specs": [], # Developer builds are not optimized. "production_release": False, @@ -60,7 +60,7 @@ _FLAVOR_METADATA = { "dev_kitkat": { "manifest": "//app:src/main/AndroidManifest.xml", "min_sdk_version": 19, - "target_sdk_version": 31, + "target_sdk_version": 33, "multidex": "manual_main_dex", "main_dex_list": _MAIN_DEX_LIST_TARGET_KITKAT, "proguard_specs": [], # Developer builds are not optimized. @@ -75,7 +75,7 @@ _FLAVOR_METADATA = { "alpha": { "manifest": "//app:src/main/AndroidManifest.xml", "min_sdk_version": 21, - "target_sdk_version": 31, + "target_sdk_version": 33, "multidex": "native", "proguard_specs": _PRODUCTION_PROGUARD_SPECS, "production_release": True, @@ -89,7 +89,7 @@ _FLAVOR_METADATA = { "alpha_kitkat": { "manifest": "//app:src/main/AndroidManifest.xml", "min_sdk_version": 19, - "target_sdk_version": 31, + "target_sdk_version": 33, "multidex": "manual_main_dex", "main_dex_list": _MAIN_DEX_LIST_TARGET_KITKAT, "proguard_specs": [], @@ -104,7 +104,7 @@ _FLAVOR_METADATA = { "alpha_kenya": { "manifest": "//app:src/main/AndroidManifest.xml", "min_sdk_version": 21, - "target_sdk_version": 31, + "target_sdk_version": 33, "multidex": "native", "proguard_specs": _PRODUCTION_PROGUARD_SPECS, "production_release": True, @@ -118,7 +118,7 @@ _FLAVOR_METADATA = { "beta": { "manifest": "//app:src/main/AndroidManifest.xml", "min_sdk_version": 21, - "target_sdk_version": 31, + "target_sdk_version": 33, "multidex": "native", "proguard_specs": _PRODUCTION_PROGUARD_SPECS, "production_release": True, @@ -132,7 +132,7 @@ _FLAVOR_METADATA = { "ga": { "manifest": "//app:src/main/AndroidManifest.xml", "min_sdk_version": 21, - "target_sdk_version": 31, + "target_sdk_version": 33, "multidex": "native", "proguard_specs": _PRODUCTION_PROGUARD_SPECS, "production_release": True, diff --git a/config/src/java/org/oppia/android/config/AndroidManifest.xml b/config/src/java/org/oppia/android/config/AndroidManifest.xml index 1d3f57544f1..123ff9bc501 100644 --- a/config/src/java/org/oppia/android/config/AndroidManifest.xml +++ b/config/src/java/org/oppia/android/config/AndroidManifest.xml @@ -1,5 +1,5 @@ - + diff --git a/data/build.gradle b/data/build.gradle index d51015b8c24..3f02df5e6d0 100644 --- a/data/build.gradle +++ b/data/build.gradle @@ -4,12 +4,12 @@ apply plugin: 'kotlin-android-extensions' apply plugin: 'kotlin-kapt' android { - compileSdkVersion 31 + compileSdkVersion 33 buildToolsVersion "29.0.2" defaultConfig { minSdkVersion 19 - targetSdkVersion 31 + targetSdkVersion 33 versionCode 1 versionName "1.0" testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" diff --git a/data/src/test/resources/robolectric.properties b/data/src/test/resources/robolectric.properties index e16d090bdb2..19419ffe423 100644 --- a/data/src/test/resources/robolectric.properties +++ b/data/src/test/resources/robolectric.properties @@ -1,3 +1,3 @@ # data/src/test/resources/robolectric.properties -# TODO(#4748): Remove the need for this file after upgrading Robolectric tests to API 31 +# TODO(#4748): Remove the need for this file after upgrading Robolectric tests to API 33 sdk=30 diff --git a/domain/build.gradle b/domain/build.gradle index b0e246d6e27..f119c351808 100644 --- a/domain/build.gradle +++ b/domain/build.gradle @@ -4,12 +4,12 @@ apply plugin: 'kotlin-android-extensions' apply plugin: 'kotlin-kapt' android { - compileSdkVersion 31 + compileSdkVersion 33 buildToolsVersion "29.0.2" defaultConfig { minSdkVersion 19 - targetSdkVersion 31 + targetSdkVersion 33 versionCode 1 versionName "1.0" javaCompileOptions { diff --git a/domain/src/main/AndroidManifest.xml b/domain/src/main/AndroidManifest.xml index 483a96cc057..ea5a0a7a495 100644 --- a/domain/src/main/AndroidManifest.xml +++ b/domain/src/main/AndroidManifest.xml @@ -1,5 +1,5 @@ - + diff --git a/domain/src/test/resources/robolectric.properties b/domain/src/test/resources/robolectric.properties index eb1a9bb98c2..cedb3da0a90 100644 --- a/domain/src/test/resources/robolectric.properties +++ b/domain/src/test/resources/robolectric.properties @@ -1,3 +1,3 @@ # domain/src/test/resources/robolectric.properties -# TODO(#4748): Remove the need for this file after upgrading Robolectric tests to API 31 +# TODO(#4748): Remove the need for this file after upgrading Robolectric tests to API 33 sdk=30 diff --git a/instrumentation/BUILD.bazel b/instrumentation/BUILD.bazel index 22a68271e54..03c56337516 100644 --- a/instrumentation/BUILD.bazel +++ b/instrumentation/BUILD.bazel @@ -18,7 +18,7 @@ android_binary( manifest_values = { "applicationId": "org.oppia.android", "minSdkVersion": "19", - "targetSdkVersion": "31", + "targetSdkVersion": "33", "versionCode": "0", "versionName": "0.1-test", }, diff --git a/testing/build.gradle b/testing/build.gradle index 65e7a17590e..3116f0df05c 100644 --- a/testing/build.gradle +++ b/testing/build.gradle @@ -4,12 +4,12 @@ apply plugin: 'kotlin-android-extensions' apply plugin: 'kotlin-kapt' android { - compileSdkVersion 31 + compileSdkVersion 33 buildToolsVersion "29.0.2" defaultConfig { minSdkVersion 19 - targetSdkVersion 31 + targetSdkVersion 33 versionCode 1 versionName "1.0" } diff --git a/testing/src/test/resources/robolectric.properties b/testing/src/test/resources/robolectric.properties index df7e21b43c0..12d726938f8 100644 --- a/testing/src/test/resources/robolectric.properties +++ b/testing/src/test/resources/robolectric.properties @@ -1,3 +1,3 @@ # testing/src/test/resources/robolectric.properties -# TODO(#4748): Remove the need for this file after upgrading Robolectric tests to API 31 +# TODO(#4748): Remove the need for this file after upgrading Robolectric tests to API 33 sdk=30 diff --git a/utility/build.gradle b/utility/build.gradle index 5039f921dd3..39589a8692b 100644 --- a/utility/build.gradle +++ b/utility/build.gradle @@ -4,12 +4,12 @@ apply plugin: 'kotlin-android-extensions' apply plugin: 'kotlin-kapt' android { - compileSdkVersion 31 + compileSdkVersion 33 buildToolsVersion "29.0.2" defaultConfig { minSdkVersion 19 - targetSdkVersion 31 + targetSdkVersion 33 versionCode 1 versionName "1.0" javaCompileOptions { diff --git a/utility/src/main/AndroidManifest.xml b/utility/src/main/AndroidManifest.xml index 5a8bce58b67..d06626d50ac 100644 --- a/utility/src/main/AndroidManifest.xml +++ b/utility/src/main/AndroidManifest.xml @@ -1,4 +1,4 @@ - + diff --git a/utility/src/test/resources/robolectric.properties b/utility/src/test/resources/robolectric.properties index 467b28a73b9..7b9532ffcbf 100644 --- a/utility/src/test/resources/robolectric.properties +++ b/utility/src/test/resources/robolectric.properties @@ -1,3 +1,3 @@ # utility/src/test/resources/robolectric.properties -# TODO(#4748): Remove the need for this file after upgrading Robolectric tests to API 31 +# TODO(#4748): Remove the need for this file after upgrading Robolectric tests to API 33 sdk=30