From ab5e1801399c19a13d77f417a8b297f39824e430 Mon Sep 17 00:00:00 2001 From: ykhung Date: Thu, 15 Apr 2021 22:47:15 +0800 Subject: [PATCH] Refine logic and add simple test for BatteryChartPreferenceController Bug: 177406865 Test: make SettingsRoboTests Test: make SettingsGoogleRoboTests Change-Id: Ie218cf967c6f30c6eadcdfe6bfd3f37ccdc2276e --- .../BatteryChartPreferenceController.java | 80 +++++--- .../settings/fuelgauge/BatteryDiffEntry.java | 2 +- .../settings/fuelgauge/BatteryEntry.java | 2 +- .../BatteryChartPreferenceControllerTest.java | 194 ++++++++++++++++++ .../settings/fuelgauge/ConvertUtilsTest.java | 28 +-- 5 files changed, 261 insertions(+), 45 deletions(-) create mode 100644 tests/robotests/src/com/android/settings/fuelgauge/BatteryChartPreferenceControllerTest.java diff --git a/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java b/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java index b5cc6ea07b5..8b1c6b5e8bb 100644 --- a/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java +++ b/src/com/android/settings/fuelgauge/BatteryChartPreferenceController.java @@ -50,16 +50,15 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll private static final int CHART_LEVEL_ARRAY_SIZE = 13; @VisibleForTesting - PreferenceGroup mAppListPrefGroup; + Map> mBatteryIndexedMap; - private Context mPrefContext; - private BatteryChartView mBatteryChartView; - // Battery history relative data. - private int[] mBatteryHistoryLevels; - private long[] mBatteryHistoryKeys; - private Map> mBatteryHistoryMap; + @VisibleForTesting Context mPrefContext; + @VisibleForTesting PreferenceGroup mAppListPrefGroup; + @VisibleForTesting BatteryChartView mBatteryChartView; - private int mTrapezoidIndex = BatteryChartView.SELECTED_INDEX_INVALID; + @VisibleForTesting int[] mBatteryHistoryLevels; + @VisibleForTesting long[] mBatteryHistoryKeys; + @VisibleForTesting int mTrapezoidIndex = BatteryChartView.SELECTED_INDEX_INVALID; private final String mPreferenceKey; private final SettingsActivity mActivity; @@ -84,6 +83,9 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll @Override public void onDestroy() { + if (mActivity.isChangingConfigurations()) { + BatteryDiffEntry.clearCache(); + } } @Override @@ -110,30 +112,34 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll @Override public void onSelect(int trapezoidIndex) { - Log.d(TAG, "onSelect:" + trapezoidIndex); - refreshUi(trapezoidIndex); + Log.d(TAG, "onChartSelect:" + trapezoidIndex); + refreshUi(trapezoidIndex, /*isForce=*/ false); } void setBatteryHistoryMap(Map> batteryHistoryMap) { - // Assumes all timestamp data is consecutive and aligns to hourly time slot. - mBatteryHistoryMap = batteryHistoryMap; + // Resets all battery history data relative variables. + if (batteryHistoryMap == null) { + mBatteryIndexedMap = null; + mBatteryHistoryKeys = null; + mBatteryHistoryLevels = null; + return; + } // Generates battery history keys. final List batteryHistoryKeyList = - new ArrayList(mBatteryHistoryMap.keySet()); - // Sorts all timestamp keys ordered by ASC from the map keys. + new ArrayList(batteryHistoryMap.keySet()); Collections.sort(batteryHistoryKeyList); mBatteryHistoryKeys = new long[CHART_KEY_ARRAY_SIZE]; - final int elementSize = Math.min( - batteryHistoryKeyList.size(), CHART_KEY_ARRAY_SIZE); + final int elementSize = Math.min(batteryHistoryKeyList.size(), CHART_KEY_ARRAY_SIZE); final int offset = CHART_KEY_ARRAY_SIZE - elementSize; for (int index = 0; index < elementSize; index++) { mBatteryHistoryKeys[index + offset] = batteryHistoryKeyList.get(index); } + // Generates the battery history levels. mBatteryHistoryLevels = new int[CHART_LEVEL_ARRAY_SIZE]; for (int index = 0; index < CHART_LEVEL_ARRAY_SIZE; index++) { final Long timestamp = Long.valueOf(mBatteryHistoryKeys[index * 2]); - final List entryList = mBatteryHistoryMap.get(timestamp); + final List entryList = batteryHistoryMap.get(timestamp); if (entryList != null && !entryList.isEmpty()) { // All battery levels are the same in the same timestamp snapshot. mBatteryHistoryLevels[index] = entryList.get(0).mBatteryLevel; @@ -142,10 +148,15 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll ConvertUtils.utcToLocalTime(timestamp)); } } - if (mBatteryChartView != null) { - mBatteryChartView.setLevels(mBatteryHistoryLevels); - } - Log.d(TAG, String.format("setBatteryHistoryMap() size=%d\nkeys=%s\nlevels=%s", + // Generates indexed usage map for chart. + mBatteryIndexedMap = + ConvertUtils.getIndexedUsageMap( + mPrefContext, /*timeSlotSize=*/ CHART_LEVEL_ARRAY_SIZE - 1, + mBatteryHistoryKeys, batteryHistoryMap); + forceRefreshUi(); + + Log.d(TAG, String.format( + "setBatteryHistoryMap() size=%d\nkeys=%s\nlevels=%s", batteryHistoryKeyList.size(), utcToLocalTime(mBatteryHistoryKeys), Arrays.toString(mBatteryHistoryLevels))); @@ -154,20 +165,29 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll void setBatteryChartView(BatteryChartView batteryChartView) { mBatteryChartView = batteryChartView; mBatteryChartView.setOnSelectListener(this); - if (mBatteryHistoryLevels != null) { - mBatteryChartView.setLevels(mBatteryHistoryLevels); - } + forceRefreshUi(); } - private void refreshUi(int trapezoidIndex) { + private void forceRefreshUi() { + final int refreshIndex = + mTrapezoidIndex == BatteryChartView.SELECTED_INDEX_INVALID + ? BatteryChartView.SELECTED_INDEX_ALL + : mTrapezoidIndex; + refreshUi(refreshIndex, /*isForce=*/ true); + } + + @VisibleForTesting + boolean refreshUi(int trapezoidIndex, boolean isForce) { // Invalid refresh condition. - if (mBatteryHistoryMap == null || mBatteryChartView == null || - mTrapezoidIndex == trapezoidIndex) { - return; + if (mBatteryIndexedMap == null + || mBatteryChartView == null + || (mTrapezoidIndex == trapezoidIndex && !isForce)) { + return false; } mTrapezoidIndex = trapezoidIndex; - Log.d(TAG, String.format("refreshUi: index=%d size=%d", - mTrapezoidIndex, mBatteryHistoryMap.size())); + Log.d(TAG, String.format("refreshUi: index=%d batteryIndexedMap.size=%d", + mTrapezoidIndex, mBatteryIndexedMap.size())); + return true; } private static String utcToLocalTime(long[] timestamps) { diff --git a/src/com/android/settings/fuelgauge/BatteryDiffEntry.java b/src/com/android/settings/fuelgauge/BatteryDiffEntry.java index da3825f0b80..c57ee6fd869 100644 --- a/src/com/android/settings/fuelgauge/BatteryDiffEntry.java +++ b/src/com/android/settings/fuelgauge/BatteryDiffEntry.java @@ -200,7 +200,7 @@ public final class BatteryDiffEntry { final BatteryEntry.NameAndIcon nameAndIcon = BatteryEntry.loadNameAndIcon( - mContext, uid, /*uid=*/ null, /*batteryEntry=*/ null, + mContext, uid, /*handler=*/ null, /*batteryEntry=*/ null, packageName, mAppLabel, mAppIcon); // Clears BatteryEntry internal cache since we will have another one. BatteryEntry.clearUidCache(); diff --git a/src/com/android/settings/fuelgauge/BatteryEntry.java b/src/com/android/settings/fuelgauge/BatteryEntry.java index 2d9f715fca9..151284a30ae 100644 --- a/src/com/android/settings/fuelgauge/BatteryEntry.java +++ b/src/com/android/settings/fuelgauge/BatteryEntry.java @@ -385,7 +385,7 @@ public class BatteryEntry { sUidCache.put(uidString, utd); if (handler != null) { - handler.sendMessage(sHandler.obtainMessage(MSG_UPDATE_NAME_ICON, batteryEntry)); + handler.sendMessage(handler.obtainMessage(MSG_UPDATE_NAME_ICON, batteryEntry)); } return new NameAndIcon(name, defaultPackageName, icon, /*iconId=*/ 0); } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryChartPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryChartPreferenceControllerTest.java new file mode 100644 index 00000000000..7d649608861 --- /dev/null +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryChartPreferenceControllerTest.java @@ -0,0 +1,194 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.fuelgauge; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.Matchers.anyInt; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + +import android.content.Context; +import android.content.ContentValues; +import android.content.pm.PackageManager; + +import androidx.preference.PreferenceGroup; + +import com.android.settings.R; +import com.android.settings.SettingsActivity; +import com.android.settings.core.InstrumentedPreferenceFragment; +import com.android.settings.testutils.FakeFeatureFactory; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; + +import java.util.Arrays; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +@RunWith(RobolectricTestRunner.class) +public final class BatteryChartPreferenceControllerTest { + + @Mock private InstrumentedPreferenceFragment mFragment; + @Mock private SettingsActivity mSettingsActivity; + @Mock private PreferenceGroup mAppListGroup; + @Mock private PackageManager mPackageManager; + @Mock private BatteryChartView mBatteryChartView; + + private Context mContext; + private BatteryChartPreferenceController mBatteryChartPreferenceController; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + mContext = spy(RuntimeEnvironment.application); + mBatteryChartPreferenceController = + new BatteryChartPreferenceController( + mContext, "app_list", /*lifecycle=*/ null, + mSettingsActivity, mFragment); + mBatteryChartPreferenceController.mPrefContext = mContext; + mBatteryChartPreferenceController.mAppListPrefGroup = mAppListGroup; + mBatteryChartPreferenceController.mBatteryChartView = mBatteryChartView; + // Adds fake testing data. + BatteryDiffEntry.sResourceCache.put( + "fakeBatteryDiffEntryKey", + new BatteryEntry.NameAndIcon("fakeName", /*icon=*/ null, /*iconId=*/ 1)); + mBatteryChartPreferenceController.setBatteryHistoryMap( + createBatteryHistoryMap(/*size=*/ 5)); + } + + @Test + public void testOnDestroy_activityIsChanging_clearBatteryEntryCache() { + doReturn(true).when(mSettingsActivity).isChangingConfigurations(); + // Ensures the testing environment is correct. + assertThat(BatteryDiffEntry.sResourceCache).hasSize(1); + + mBatteryChartPreferenceController.onDestroy(); + assertThat(BatteryDiffEntry.sResourceCache).isEmpty(); + } + + @Test + public void testOnDestroy_activityIsNotChanging_notClearBatteryEntryCache() { + doReturn(false).when(mSettingsActivity).isChangingConfigurations(); + // Ensures the testing environment is correct. + assertThat(BatteryDiffEntry.sResourceCache).hasSize(1); + + mBatteryChartPreferenceController.onDestroy(); + assertThat(BatteryDiffEntry.sResourceCache).isNotEmpty(); + } + + @Test + public void testSetBatteryHistoryMap_createExpectedKeysAndLevels() { + mBatteryChartPreferenceController.setBatteryHistoryMap( + createBatteryHistoryMap(/*size=*/ 5)); + + // Verifies the created battery keys array. + for (int index = 0; index < 25; index++) { + assertThat(mBatteryChartPreferenceController.mBatteryHistoryKeys[index]) + // These values is are calculated by hand from createBatteryHistoryMap(). + .isEqualTo(index < 20 ? 0 : (index - 20 + 1)); + } + // Verifies the created battery levels array. + for (int index = 0; index < 13; index++) { + assertThat(mBatteryChartPreferenceController.mBatteryHistoryLevels[index]) + // These values is are calculated by hand from createBatteryHistoryMap(). + .isEqualTo(index < 10 ? 0 : (100 - (index - 10) * 2)); + } + assertThat(mBatteryChartPreferenceController.mBatteryIndexedMap).hasSize(13); + } + + @Test + public void testSetBatteryHistoryMap_largeSize_createExpectedKeysAndLevels() { + mBatteryChartPreferenceController.setBatteryHistoryMap( + createBatteryHistoryMap(/*size=*/ 25)); + + // Verifies the created battery keys array. + for (int index = 0; index < 25; index++) { + assertThat(mBatteryChartPreferenceController.mBatteryHistoryKeys[index]) + // These values is are calculated by hand from createBatteryHistoryMap(). + .isEqualTo(index + 1); + } + // Verifies the created battery levels array. + for (int index = 0; index < 13; index++) { + assertThat(mBatteryChartPreferenceController.mBatteryHistoryLevels[index]) + // These values is are calculated by hand from createBatteryHistoryMap(). + .isEqualTo(100 - index * 2); + } + assertThat(mBatteryChartPreferenceController.mBatteryIndexedMap).hasSize(13); + } + + @Test + public void testRefreshUi_batteryIndexedMapIsNull_ignoreRefresh() { + mBatteryChartPreferenceController.setBatteryHistoryMap(null); + assertThat(mBatteryChartPreferenceController.refreshUi( + /*trapezoidIndex=*/ 1, /*isForce=*/ false)).isFalse(); + } + + @Test + public void testRefreshUi_batteryChartViewIsNull_ignoreRefresh() { + mBatteryChartPreferenceController.mBatteryChartView = null; + assertThat(mBatteryChartPreferenceController.refreshUi( + /*trapezoidIndex=*/ 1, /*isForce=*/ false)).isFalse(); + } + + @Test + public void testRefreshUi_trapezoidIndexIsNotChanged_ignoreRefresh() { + final int trapezoidIndex = 1; + mBatteryChartPreferenceController.mTrapezoidIndex = trapezoidIndex; + assertThat(mBatteryChartPreferenceController.refreshUi( + trapezoidIndex, /*isForce=*/ false)).isFalse(); + } + + @Test + public void testRefreshUi_forceUpdate_refreshUi() { + final int trapezoidIndex = 1; + mBatteryChartPreferenceController.mTrapezoidIndex = trapezoidIndex; + assertThat(mBatteryChartPreferenceController.refreshUi( + trapezoidIndex, /*isForce=*/ true)).isTrue(); + } + + @Test + public void testForceRefreshUi_updateTrapezoidIndexIntoSelectAll() { + mBatteryChartPreferenceController.mTrapezoidIndex = + BatteryChartView.SELECTED_INDEX_INVALID; + mBatteryChartPreferenceController.setBatteryHistoryMap( + createBatteryHistoryMap(/*size=*/ 25)); + + + assertThat(mBatteryChartPreferenceController.mTrapezoidIndex) + .isEqualTo(BatteryChartView.SELECTED_INDEX_ALL); + } + + private Map> createBatteryHistoryMap(int size) { + final Map> batteryHistoryMap = new HashMap<>(); + for (int index = 0; index < size; index++) { + final ContentValues values = new ContentValues(); + values.put("batteryLevel", Integer.valueOf(100 - index)); + final BatteryHistEntry entry = new BatteryHistEntry(values); + batteryHistoryMap.put(Long.valueOf(index + 1), Arrays.asList(entry)); + } + return batteryHistoryMap; + } +} diff --git a/tests/robotests/src/com/android/settings/fuelgauge/ConvertUtilsTest.java b/tests/robotests/src/com/android/settings/fuelgauge/ConvertUtilsTest.java index 7ce488d765c..cfd23c9a410 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/ConvertUtilsTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/ConvertUtilsTest.java @@ -199,16 +199,16 @@ public final class ConvertUtilsTest { createBatteryHistEntry( "package2", "label2", 15.0, 2L, 25L, 35L), createBatteryHistEntry( - "package3", "label3", 5.0, 2L, 5L, 5L))); + "package3", "label3", 5.0, 3L, 5L, 5L))); batteryHistoryMap.put( Long.valueOf(batteryHistoryKeys[4]), Arrays.asList( createBatteryHistEntry( "package2", "label2", 30.0, 2L, 30L, 40L), createBatteryHistEntry( - "package2", "label2", 75.0, 3L, 40L, 50L), + "package2", "label2", 75.0, 4L, 40L, 50L), createBatteryHistEntry( - "package3", "label3", 5.0, 2L, 5L, 5L))); + "package3", "label3", 5.0, 3L, 5L, 5L))); final Map> resultMap = ConvertUtils.getIndexedUsageMap( @@ -222,28 +222,30 @@ public final class ConvertUtilsTest { // Verifies the second timestamp result. entryList = resultMap.get(Integer.valueOf(1)); assertThat(entryList).hasSize(3); - assertBatteryDiffEntry(entryList.get(0), 5, 5L, 5L); - assertBatteryDiffEntry(entryList.get(1), 75, 40L, 50L); - assertBatteryDiffEntry(entryList.get(2), 20, 15L, 15L); + assertBatteryDiffEntry(entryList.get(1), 5, 5L, 5L); + assertBatteryDiffEntry(entryList.get(2), 75, 40L, 50L); + assertBatteryDiffEntry(entryList.get(0), 20, 15L, 15L); // Verifies the last 24 hours aggregate result. entryList = resultMap.get(Integer.valueOf(-1)); assertThat(entryList).hasSize(3); - assertBatteryDiffEntry(entryList.get(0), 4, 5L, 5L); - assertBatteryDiffEntry(entryList.get(1), 68, 40L, 50L); - assertBatteryDiffEntry(entryList.get(2), 27, 30L, 40L); + assertBatteryDiffEntry(entryList.get(1), 4, 5L, 5L); + assertBatteryDiffEntry(entryList.get(2), 68, 40L, 50L); + assertBatteryDiffEntry(entryList.get(0), 27, 30L, 40L); } private static BatteryHistEntry createBatteryHistEntry( String packageName, String appLabel, double consumePower, - long userId, long foregroundUsageTimeInMs, long backgroundUsageTimeInMs) { + long uid, long foregroundUsageTimeInMs, long backgroundUsageTimeInMs) { // Only insert required fields. final ContentValues values = new ContentValues(); values.put("packageName", packageName); values.put("appLabel", appLabel); - values.put("userId", userId); + values.put("uid", Long.valueOf(uid)); + values.put("consumerType", + Integer.valueOf(ConvertUtils.CONSUMER_TYPE_UID_BATTERY)); values.put("consumePower", consumePower); - values.put("foregroundUsageTimeInMs", foregroundUsageTimeInMs); - values.put("backgroundUsageTimeInMs", backgroundUsageTimeInMs); + values.put("foregroundUsageTimeInMs", Long.valueOf(foregroundUsageTimeInMs)); + values.put("backgroundUsageTimeInMs", Long.valueOf(backgroundUsageTimeInMs)); return new BatteryHistEntry(values); }