From ca0542d2d726517c7f37d46814948171fe985393 Mon Sep 17 00:00:00 2001
From: Chaohui Wang <chaohuiw@google.com>
Date: Wed, 17 Jan 2024 17:55:16 +0800
Subject: [PATCH] Avoid empty line for storage summary

Bug: 292036686
Test: manual - on All Apps
Test: unit test
Change-Id: Id5069b8172191f3277b825f82407551f6eb4e202
---
 .../android/settings/spa/app/AllAppList.kt    |  8 +++-
 .../settings/spa/app/AllAppListTest.kt        | 38 +++++++++++++++++--
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/src/com/android/settings/spa/app/AllAppList.kt b/src/com/android/settings/spa/app/AllAppList.kt
index 5b13211adda..5e0ac9513dd 100644
--- a/src/com/android/settings/spa/app/AllAppList.kt
+++ b/src/com/android/settings/spa/app/AllAppList.kt
@@ -34,6 +34,7 @@ import com.android.settingslib.spa.framework.util.mapItem
 import com.android.settingslib.spa.widget.preference.Preference
 import com.android.settingslib.spa.widget.preference.PreferenceModel
 import com.android.settingslib.spa.widget.ui.SpinnerOption
+import com.android.settingslib.spaprivileged.framework.compose.getPlaceholder
 import com.android.settingslib.spaprivileged.model.app.AppListModel
 import com.android.settingslib.spaprivileged.model.app.AppRecord
 import com.android.settingslib.spaprivileged.model.app.installed
@@ -131,7 +132,11 @@ class AllAppListModel(
     override fun getSummary(option: Int, record: AppRecordWithSize): () -> String {
         val storageSummary = record.app.getStorageSummary()
         return {
-            val summaryList = mutableListOf(storageSummary.value)
+            val summaryList = mutableListOf<String>()
+            val storageSummaryValue = storageSummary.value
+            if (storageSummaryValue.isNotBlank()) {
+                summaryList += storageSummaryValue
+            }
             when {
                 !record.app.installed && !record.app.isArchived -> {
                     summaryList += context.getString(R.string.not_installed)
@@ -142,6 +147,7 @@ class AllAppListModel(
                 }
             }
             summaryList.joinToString(separator = System.lineSeparator())
+                .ifEmpty { context.getPlaceholder() } // Use placeholder to reduce flaky
         }
     }
 
diff --git a/tests/spa_unit/src/com/android/settings/spa/app/AllAppListTest.kt b/tests/spa_unit/src/com/android/settings/spa/app/AllAppListTest.kt
index 1a05479fda0..0ec87548407 100644
--- a/tests/spa_unit/src/com/android/settings/spa/app/AllAppListTest.kt
+++ b/tests/spa_unit/src/com/android/settings/spa/app/AllAppListTest.kt
@@ -31,6 +31,7 @@ import com.android.settings.R
 import com.android.settingslib.spa.framework.compose.stateOf
 import com.android.settingslib.spa.testutils.FakeNavControllerWrapper
 import com.android.settingslib.spa.testutils.firstWithTimeoutOrNull
+import com.android.settingslib.spaprivileged.framework.compose.getPlaceholder
 import com.android.settingslib.spaprivileged.template.app.AppListInput
 import com.android.settingslib.spaprivileged.template.app.AppListItemModel
 import com.google.common.truth.Truth.assertThat
@@ -142,7 +143,7 @@ class AllAppListTest {
     }
 
     @Test
-    fun allAppListModel_getSummary() {
+    fun listModelGetSummary_regular() {
         val listModel = AllAppListModel(context) { stateOf(SUMMARY) }
 
         lateinit var summary: () -> String
@@ -154,7 +155,19 @@ class AllAppListTest {
     }
 
     @Test
-    fun allAppListModel_getSummaryWhenDisabled() {
+    fun listModelGetSummary_emptyStorage() {
+        val listModel = AllAppListModel(context) { stateOf("") }
+
+        lateinit var summary: () -> String
+        composeTestRule.setContent {
+            summary = listModel.getSummary(option = 0, record = AppRecordWithSize(app = APP))
+        }
+
+        assertThat(summary()).isEqualTo(context.getPlaceholder())
+    }
+
+    @Test
+    fun listModelGetSummary_disabled() {
         val listModel = AllAppListModel(context) { stateOf(SUMMARY) }
         val disabledApp = ApplicationInfo().apply {
             packageName = PACKAGE_NAME
@@ -172,7 +185,26 @@ class AllAppListTest {
     }
 
     @Test
-    fun allAppListModel_getSummaryWhenNotInstalled() {
+    fun listModelGetSummary_emptyStorageAndDisabled() {
+        val listModel = AllAppListModel(context) { stateOf("") }
+        val disabledApp = ApplicationInfo().apply {
+            packageName = PACKAGE_NAME
+            flags = ApplicationInfo.FLAG_INSTALLED
+            enabled = false
+        }
+
+        lateinit var summary: () -> String
+        composeTestRule.setContent {
+            summary =
+                listModel.getSummary(option = 0, record = AppRecordWithSize(app = disabledApp))
+        }
+
+        assertThat(summary())
+            .isEqualTo(context.getString(com.android.settingslib.R.string.disabled))
+    }
+
+    @Test
+    fun listModelGetSummary_notInstalled() {
         val listModel = AllAppListModel(context) { stateOf(SUMMARY) }
         val notInstalledApp = ApplicationInfo().apply {
             packageName = PACKAGE_NAME