From 88d876318c47931289dcc67764705c4069555346 Mon Sep 17 00:00:00 2001 From: Zaiyue Xue Date: Fri, 14 Oct 2022 13:51:36 +0800 Subject: [PATCH 1/2] Fix broken battery usage Robolectric tests Bug: 248686898 Test: presubmit Change-Id: I4f09f9e5af57a62249970cc2170b60dab267488e Merged-In: I4f09f9e5af57a62249970cc2170b60dab267488e --- .../BatteryChartPreferenceControllerTest.java | 29 +++++++++++++- .../batteryusage/DataProcessorTest.java | 40 ++++++++++++++----- 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java index 26e0f5074ed..4e552b941c0 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java @@ -18,6 +18,8 @@ package com.android.settings.fuelgauge.batteryusage; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.anyFloat; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.any; import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.atLeastOnce; @@ -36,6 +38,7 @@ import android.os.Bundle; import android.os.LocaleList; import android.text.format.DateUtils; import android.view.View; +import android.view.ViewPropertyAnimator; import android.widget.LinearLayout; import androidx.preference.Preference; @@ -85,6 +88,8 @@ public final class BatteryChartPreferenceControllerTest { @Mock private BatteryChartView mHourlyChartView; @Mock + private ViewPropertyAnimator mViewPropertyAnimator; + @Mock private PowerGaugePreference mPowerGaugePreference; @Mock private BatteryUtils mBatteryUtils; @@ -116,6 +121,7 @@ public final class BatteryChartPreferenceControllerTest { .when(mFeatureFactory.powerUsageFeatureProvider) .getHideApplicationEntries(mContext); doReturn(mLayoutParams).when(mDailyChartView).getLayoutParams(); + setupHourlyChartViewAnimationMock(); mBatteryChartPreferenceController = createController(); mBatteryChartPreferenceController.mPrefContext = mContext; mBatteryChartPreferenceController.mAppListPrefGroup = mAppListGroup; @@ -174,11 +180,15 @@ public final class BatteryChartPreferenceControllerTest { @Test public void setBatteryChartViewModel_6Hours() { + reset(mDailyChartView); reset(mHourlyChartView); + setupHourlyChartViewAnimationMock(); + mBatteryChartPreferenceController.setBatteryHistoryMap(createBatteryHistoryMap(6)); verify(mDailyChartView, atLeastOnce()).setVisibility(View.GONE); verify(mHourlyChartView, atLeastOnce()).setVisibility(View.VISIBLE); + verify(mViewPropertyAnimator, atLeastOnce()).alpha(1f); // Ignore fast refresh ui from the data processor callback. verify(mHourlyChartView, atLeast(0)).setViewModel(null); verify(mHourlyChartView, atLeastOnce()).setViewModel(new BatteryChartViewModel( @@ -192,6 +202,10 @@ public final class BatteryChartPreferenceControllerTest { @Test public void setBatteryChartViewModel_60Hours() { + reset(mDailyChartView); + reset(mHourlyChartView); + setupHourlyChartViewAnimationMock(); + BatteryChartViewModel expectedDailyViewModel = new BatteryChartViewModel( List.of(100, 83, 59, 41), // "Sat", "Sun", "Mon", "Mon" @@ -205,16 +219,18 @@ public final class BatteryChartPreferenceControllerTest { mBatteryChartPreferenceController.setBatteryHistoryMap(createBatteryHistoryMap(60)); verify(mDailyChartView, atLeastOnce()).setVisibility(View.VISIBLE); - verify(mHourlyChartView, atLeastOnce()).setVisibility(View.GONE); + verify(mViewPropertyAnimator, atLeastOnce()).alpha(0f); verify(mDailyChartView).setViewModel(expectedDailyViewModel); reset(mDailyChartView); reset(mHourlyChartView); + setupHourlyChartViewAnimationMock(); doReturn(mLayoutParams).when(mDailyChartView).getLayoutParams(); mBatteryChartPreferenceController.mDailyChartIndex = 0; mBatteryChartPreferenceController.refreshUi(); verify(mDailyChartView).setVisibility(View.VISIBLE); verify(mHourlyChartView).setVisibility(View.VISIBLE); + verify(mViewPropertyAnimator, atLeastOnce()).alpha(1f); expectedDailyViewModel.setSelectedIndex(0); verify(mDailyChartView).setViewModel(expectedDailyViewModel); @@ -234,12 +250,14 @@ public final class BatteryChartPreferenceControllerTest { reset(mDailyChartView); reset(mHourlyChartView); + setupHourlyChartViewAnimationMock(); doReturn(mLayoutParams).when(mDailyChartView).getLayoutParams(); mBatteryChartPreferenceController.mDailyChartIndex = 1; mBatteryChartPreferenceController.mHourlyChartIndex = 6; mBatteryChartPreferenceController.refreshUi(); verify(mDailyChartView).setVisibility(View.VISIBLE); verify(mHourlyChartView).setVisibility(View.VISIBLE); + verify(mViewPropertyAnimator, atLeastOnce()).alpha(1f); expectedDailyViewModel.setSelectedIndex(1); verify(mDailyChartView).setViewModel(expectedDailyViewModel); BatteryChartViewModel expectedHourlyViewModel = new BatteryChartViewModel( @@ -264,6 +282,7 @@ public final class BatteryChartPreferenceControllerTest { reset(mDailyChartView); reset(mHourlyChartView); + setupHourlyChartViewAnimationMock(); doReturn(mLayoutParams).when(mDailyChartView).getLayoutParams(); mBatteryChartPreferenceController.mDailyChartIndex = 2; mBatteryChartPreferenceController.mHourlyChartIndex = @@ -271,6 +290,7 @@ public final class BatteryChartPreferenceControllerTest { mBatteryChartPreferenceController.refreshUi(); verify(mDailyChartView).setVisibility(View.VISIBLE); verify(mHourlyChartView).setVisibility(View.VISIBLE); + verify(mViewPropertyAnimator, atLeastOnce()).alpha(1f); expectedDailyViewModel.setSelectedIndex(2); verify(mDailyChartView).setViewModel(expectedDailyViewModel); verify(mHourlyChartView).setViewModel(new BatteryChartViewModel( @@ -734,4 +754,11 @@ public final class BatteryChartPreferenceControllerTest { controller.mPrefContext = mContext; return controller; } + + private void setupHourlyChartViewAnimationMock() { + doReturn(mViewPropertyAnimator).when(mHourlyChartView).animate(); + doReturn(mViewPropertyAnimator).when(mViewPropertyAnimator).alpha(anyFloat()); + doReturn(mViewPropertyAnimator).when(mViewPropertyAnimator).setDuration(anyLong()); + doReturn(mViewPropertyAnimator).when(mViewPropertyAnimator).setListener(any()); + } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/DataProcessorTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/DataProcessorTest.java index c4832041314..df9d8655109 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/DataProcessorTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/DataProcessorTest.java @@ -579,9 +579,13 @@ public class DataProcessorTest { ConvertUtils.CONSUMER_TYPE_SYSTEM_BATTERY, /*consumePercentage=*/ 25.0, /*foregroundUsageTimeInMs=*/ 50, /*backgroundUsageTimeInMs=*/ 60); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, 3); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, + 3); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, 0); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, + 0); } @Test @@ -674,9 +678,13 @@ public class DataProcessorTest { assertThat(resultMap.get(0).get(0)).isNotNull(); assertThat(resultMap.get(0).get(DataProcessor.SELECTED_INDEX_ALL)).isNotNull(); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, 2); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, + 2); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, 0); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, + 0); } @Test @@ -739,9 +747,13 @@ public class DataProcessorTest { assertThat(resultMap.get(0).get(0)).isNotNull(); assertThat(resultMap.get(0).get(DataProcessor.SELECTED_INDEX_ALL)).isNotNull(); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, 1); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, + 1); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, 0); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, + 0); } @Test @@ -814,9 +826,13 @@ public class DataProcessorTest { ConvertUtils.CONSUMER_TYPE_UID_BATTERY, /*consumePercentage=*/ 50.0, /*foregroundUsageTimeInMs=*/ 10, /*backgroundUsageTimeInMs=*/ 20); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, 1); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, + 1); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, 1); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, + 1); } @Test @@ -889,9 +905,13 @@ public class DataProcessorTest { resultEntry = resultDiffData.getAppDiffEntryList().get(1); assertThat(resultEntry.mBackgroundUsageTimeInMs).isEqualTo(0); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, 2); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_SHOWN_APP_COUNT, + 2); verify(mMetricsFeatureProvider) - .action(mContext, SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, 0); + .action(mContext.getApplicationContext(), + SettingsEnums.ACTION_BATTERY_USAGE_HIDDEN_APP_COUNT, + 0); } @Test From 4fbd2064d6e835cf8e145c08d5692b67376ddbcb Mon Sep 17 00:00:00 2001 From: Zaiyue Xue Date: Tue, 25 Oct 2022 15:23:37 +0800 Subject: [PATCH 2/2] Fix b/248686898: Battery Usage list renders items on top of each other after swiping back from an app Root cause: When adding an animation to the hourly chartview which is scolled up out of the page, onBindViewHolder() will be trigered, then UI is refreshed again, and then the animation is added to the hourly charview again. This is a dead loop. This fix adds animation to the hourly chartview only if the visibility is changed. http://cs/android/packages/apps/Settings/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceController.java;l=676;rcl=9f24ea815d637a05e1b7ddf2d4372d716d749229 http://cs/android/packages/apps/Settings/src/com/android/settings/fuelgauge/batteryusage/BatteryHistoryPreference.java;l=102;rcl=9f24ea815d637a05e1b7ddf2d4372d716d749229 Bug: 248686898 Fix: 248686898 Test: manually Change-Id: I93d4d089f537515d452c1330f5d75a6726b229f8 (cherry picked from commit aabea1689896adf3799a7eeffe11795dee3bd25c) Merged-In: I93d4d089f537515d452c1330f5d75a6726b229f8 --- .../batteryusage/BatteryChartPreferenceController.java | 4 +++- .../batteryusage/BatteryChartPreferenceControllerTest.java | 4 ---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceController.java b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceController.java index 2c924885c3b..56da0f47932 100644 --- a/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceController.java +++ b/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceController.java @@ -108,6 +108,7 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll private boolean mIs24HourFormat; private boolean mIsFooterPrefAdded = false; + private boolean mHourlyChartVisible = true; private View mBatteryChartViewGroup; private View mCategoryTitleView; private PreferenceScreen mPreferenceScreen; @@ -690,9 +691,10 @@ public class BatteryChartPreferenceController extends AbstractPreferenceControll } private void animateBatteryHourlyChartView(final boolean visible) { - if (mHourlyChartView == null) { + if (mHourlyChartView == null || mHourlyChartVisible == visible) { return; } + mHourlyChartVisible = visible; if (visible) { mHourlyChartView.setVisibility(View.VISIBLE); diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java index 4e552b941c0..9fbcb1688db 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryChartPreferenceControllerTest.java @@ -187,8 +187,6 @@ public final class BatteryChartPreferenceControllerTest { mBatteryChartPreferenceController.setBatteryHistoryMap(createBatteryHistoryMap(6)); verify(mDailyChartView, atLeastOnce()).setVisibility(View.GONE); - verify(mHourlyChartView, atLeastOnce()).setVisibility(View.VISIBLE); - verify(mViewPropertyAnimator, atLeastOnce()).alpha(1f); // Ignore fast refresh ui from the data processor callback. verify(mHourlyChartView, atLeast(0)).setViewModel(null); verify(mHourlyChartView, atLeastOnce()).setViewModel(new BatteryChartViewModel( @@ -256,7 +254,6 @@ public final class BatteryChartPreferenceControllerTest { mBatteryChartPreferenceController.mHourlyChartIndex = 6; mBatteryChartPreferenceController.refreshUi(); verify(mDailyChartView).setVisibility(View.VISIBLE); - verify(mHourlyChartView).setVisibility(View.VISIBLE); verify(mViewPropertyAnimator, atLeastOnce()).alpha(1f); expectedDailyViewModel.setSelectedIndex(1); verify(mDailyChartView).setViewModel(expectedDailyViewModel); @@ -289,7 +286,6 @@ public final class BatteryChartPreferenceControllerTest { BatteryChartViewModel.SELECTED_INDEX_ALL; mBatteryChartPreferenceController.refreshUi(); verify(mDailyChartView).setVisibility(View.VISIBLE); - verify(mHourlyChartView).setVisibility(View.VISIBLE); verify(mViewPropertyAnimator, atLeastOnce()).alpha(1f); expectedDailyViewModel.setSelectedIndex(2); verify(mDailyChartView).setViewModel(expectedDailyViewModel);