Skip to content

Commit

Permalink
Fix #5137: Upgrade builds to target SDK 33 (#5222)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
BenHenning authored Nov 29, 2023
1 parent 6fec2e2 commit 1181ca2
Show file tree
Hide file tree
Showing 27 changed files with 90 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
]
]
Expand Down
2 changes: 1 addition & 1 deletion WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)

Expand Down
4 changes: 2 additions & 2 deletions app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/AppAndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.oppia.android.app.ui">
<uses-sdk android:minSdkVersion="19"
android:targetSdkVersion="31" />
android:targetSdkVersion="33" />
</manifest>
2 changes: 1 addition & 1 deletion app/src/main/DatabindingAdaptersManifest.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.oppia.android.databinding.adapters">
<uses-sdk android:minSdkVersion="19"
android:targetSdkVersion="31" />
android:targetSdkVersion="33" />
</manifest>
2 changes: 1 addition & 1 deletion app/src/main/DatabindingResourcesManifest.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.oppia.android.app.databinding">
<uses-sdk android:minSdkVersion="19"
android:targetSdkVersion="31" />
android:targetSdkVersion="33" />
</manifest>
2 changes: 1 addition & 1 deletion app/src/main/RecyclerviewAdaptersManifest.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.oppia.android.app.recyclerview.adapters">
<uses-sdk android:minSdkVersion="19"
android:targetSdkVersion="31" />
android:targetSdkVersion="33" />
</manifest>
2 changes: 1 addition & 1 deletion app/src/main/ViewModelManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.oppia.android.app.vm">
<uses-sdk android:minSdkVersion="19"
android:targetSdkVersion="31" />
android:targetSdkVersion="33" />
</manifest>
2 changes: 1 addition & 1 deletion app/src/main/ViewModelsManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.oppia.android.app.view.models">
<uses-sdk android:minSdkVersion="19"
android:targetSdkVersion="31" />
android:targetSdkVersion="33" />
</manifest>
2 changes: 1 addition & 1 deletion app/src/main/ViewsManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.oppia.android.app.views">
<uses-sdk android:minSdkVersion="19"
android:targetSdkVersion="31" />
android:targetSdkVersion="33" />
</manifest>
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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
)
}
Expand Down
2 changes: 1 addition & 1 deletion app/src/test/resources/robolectric.properties
Original file line number Diff line number Diff line change
@@ -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
14 changes: 7 additions & 7 deletions build_flavors.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand All @@ -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,
Expand All @@ -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": [],
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.oppia.android.config">

<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="31" />
<uses-sdk android:minSdkVersion="19" android:targetSdkVersion="33" />
</manifest>
4 changes: 2 additions & 2 deletions data/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion data/src/test/resources/robolectric.properties
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions domain/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion domain/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.oppia.android.domain">
<uses-sdk android:targetSdkVersion="31" />
<uses-sdk android:targetSdkVersion="33" />
</manifest>
2 changes: 1 addition & 1 deletion domain/src/test/resources/robolectric.properties
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion instrumentation/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ android_binary(
manifest_values = {
"applicationId": "org.oppia.android",
"minSdkVersion": "19",
"targetSdkVersion": "31",
"targetSdkVersion": "33",
"versionCode": "0",
"versionName": "0.1-test",
},
Expand Down
4 changes: 2 additions & 2 deletions testing/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
2 changes: 1 addition & 1 deletion testing/src/test/resources/robolectric.properties
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions utility/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion utility/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.oppia.android.util">
<uses-sdk android:targetSdkVersion="31" />
<uses-sdk android:targetSdkVersion="33" />
</manifest>
2 changes: 1 addition & 1 deletion utility/src/test/resources/robolectric.properties
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 1181ca2

Please sign in to comment.