From d4482339d5f36bf253b2007f2e3c5b8e1ce4040f Mon Sep 17 00:00:00 2001 From: Lei Yu Date: Fri, 30 Mar 2018 13:20:53 -0700 Subject: [PATCH 1/2] Update job ids in Settings Use constant integers instead of ids Bug: 77331929 Test: RunSettingsRoboTests Change-Id: Ic6def54aaae4a9ba025161340f1cee210cb2e87f --- res/values/ids.xml | 3 --- res/values/integers.xml | 22 +++++++++++++++++++ .../batterytip/AnomalyCleanupJobService.java | 2 +- .../batterytip/AnomalyConfigJobService.java | 2 +- .../AnomalyDetectionJobService.java | 2 +- .../AnomalyCleanupJobServiceTest.java | 2 +- .../AnomalyConfigJobServiceTest.java | 2 +- .../AnomalyDetectionJobServiceTest.java | 2 +- 8 files changed, 28 insertions(+), 9 deletions(-) create mode 100644 res/values/integers.xml diff --git a/res/values/ids.xml b/res/values/ids.xml index 57031e1e27b..66af163e2b8 100644 --- a/res/values/ids.xml +++ b/res/values/ids.xml @@ -18,9 +18,6 @@ --> - - - diff --git a/res/values/integers.xml b/res/values/integers.xml new file mode 100644 index 00000000000..ac9a9737bd3 --- /dev/null +++ b/res/values/integers.xml @@ -0,0 +1,22 @@ + + + + + 100 + 101 + 102 + \ No newline at end of file diff --git a/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobService.java b/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobService.java index 46744f7e7a0..0c10d72802f 100644 --- a/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobService.java +++ b/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobService.java @@ -42,7 +42,7 @@ public class AnomalyCleanupJobService extends JobService { final ComponentName component = new ComponentName(context, AnomalyCleanupJobService.class); final JobInfo.Builder jobBuilder = - new JobInfo.Builder(R.id.job_anomaly_clean_up, component) + new JobInfo.Builder(R.integer.job_anomaly_clean_up, component) .setPeriodic(CLEAN_UP_FREQUENCY_MS) .setRequiresDeviceIdle(true) .setRequiresCharging(true); diff --git a/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobService.java b/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobService.java index 1a650880eac..85865bba8f6 100644 --- a/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobService.java +++ b/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobService.java @@ -52,7 +52,7 @@ public class AnomalyConfigJobService extends JobService { final ComponentName component = new ComponentName(context, AnomalyConfigJobService.class); final JobInfo.Builder jobBuilder = - new JobInfo.Builder(R.id.job_anomaly_config_update, component) + new JobInfo.Builder(R.integer.job_anomaly_config_update, component) .setPeriodic(CONFIG_UPDATE_FREQUENCY_MS) .setRequiresDeviceIdle(true) .setRequiresCharging(true); diff --git a/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java b/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java index a12d31e5a2c..52555783df8 100644 --- a/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java +++ b/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java @@ -70,7 +70,7 @@ public class AnomalyDetectionJobService extends JobService { final ComponentName component = new ComponentName(context, AnomalyDetectionJobService.class); final JobInfo.Builder jobBuilder = - new JobInfo.Builder(R.id.job_anomaly_detection, component) + new JobInfo.Builder(R.integer.job_anomaly_detection, component) .setOverrideDeadline(MAX_DELAY_MS); if (jobScheduler.enqueue(jobBuilder.build(), new JobWorkItem(intent)) diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobServiceTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobServiceTest.java index 0e7fbc5fe2d..98e4c57a683 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobServiceTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobServiceTest.java @@ -52,7 +52,7 @@ public class AnomalyCleanupJobServiceTest { List pendingJobs = shadowJobScheduler.getAllPendingJobs(); assertEquals(1, pendingJobs.size()); JobInfo pendingJob = pendingJobs.get(0); - assertThat(pendingJob.getId()).isEqualTo(R.id.job_anomaly_clean_up); + assertThat(pendingJob.getId()).isEqualTo(R.integer.job_anomaly_clean_up); assertThat(pendingJob.getIntervalMillis()).isEqualTo(TimeUnit.DAYS.toMillis(1)); assertThat(pendingJob.isRequireDeviceIdle()).isTrue(); assertThat(pendingJob.isRequireCharging()).isTrue(); diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobServiceTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobServiceTest.java index e1b85aaddb1..5ff2c70288b 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobServiceTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobServiceTest.java @@ -79,7 +79,7 @@ public class AnomalyConfigJobServiceTest { List pendingJobs = shadowJobScheduler.getAllPendingJobs(); assertEquals(1, pendingJobs.size()); JobInfo pendingJob = pendingJobs.get(0); - assertThat(pendingJob.getId()).isEqualTo(R.id.job_anomaly_config_update); + assertThat(pendingJob.getId()).isEqualTo(R.integer.job_anomaly_config_update); assertThat(pendingJob.getIntervalMillis()).isEqualTo(TimeUnit.DAYS.toMillis(1)); assertThat(pendingJob.isRequireDeviceIdle()).isTrue(); assertThat(pendingJob.isRequireCharging()).isTrue(); diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java index efc20e957d9..0894b66719a 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java @@ -113,7 +113,7 @@ public class AnomalyDetectionJobServiceTest { assertThat(pendingJobs).hasSize(1); JobInfo pendingJob = pendingJobs.get(0); - assertThat(pendingJob.getId()).isEqualTo(R.id.job_anomaly_detection); + assertThat(pendingJob.getId()).isEqualTo(R.integer.job_anomaly_detection); assertThat(pendingJob.getMaxExecutionDelayMillis()) .isEqualTo(TimeUnit.MINUTES.toMillis(30)); } From b3087c7f1fa50449ebc75c3712bd4a9dbc872c1d Mon Sep 17 00:00:00 2001 From: Lei Yu Date: Fri, 30 Mar 2018 13:59:38 -0700 Subject: [PATCH 2/2] Make anomaly jobs persistent. Before this cl, if user turn off device everday(i.e. OTA update), the every day jobs may never have a chance to run. This cl make the jobs persistent and don't reschedule it if we already have one. In this way we can make sure the jobs roughly run everyday. Bug: 77331929 Test: RunSettingsRoboTests Change-Id: Ib497aca6b696afd9386515464845c5b0dd3c1207 --- .../batterytip/AnomalyCleanupJobService.java | 9 +++++-- .../batterytip/AnomalyConfigJobService.java | 9 +++++-- .../AnomalyCleanupJobServiceTest.java | 27 +++++++++++++++++-- .../AnomalyConfigJobServiceTest.java | 24 ++++++++++++++--- 4 files changed, 60 insertions(+), 9 deletions(-) diff --git a/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobService.java b/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobService.java index 0c10d72802f..9e57433bcb7 100644 --- a/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobService.java +++ b/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobService.java @@ -45,9 +45,14 @@ public class AnomalyCleanupJobService extends JobService { new JobInfo.Builder(R.integer.job_anomaly_clean_up, component) .setPeriodic(CLEAN_UP_FREQUENCY_MS) .setRequiresDeviceIdle(true) - .setRequiresCharging(true); + .setRequiresCharging(true) + .setPersisted(true); + final JobInfo pending = jobScheduler.getPendingJob(R.integer.job_anomaly_clean_up); - if (jobScheduler.schedule(jobBuilder.build()) != JobScheduler.RESULT_SUCCESS) { + // Don't schedule it if it already exists, to make sure it runs periodically even after + // reboot + if (pending == null && jobScheduler.schedule(jobBuilder.build()) + != JobScheduler.RESULT_SUCCESS) { Log.i(TAG, "Anomaly clean up job service schedule failed."); } } diff --git a/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobService.java b/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobService.java index 85865bba8f6..98eb23e3632 100644 --- a/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobService.java +++ b/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobService.java @@ -55,9 +55,14 @@ public class AnomalyConfigJobService extends JobService { new JobInfo.Builder(R.integer.job_anomaly_config_update, component) .setPeriodic(CONFIG_UPDATE_FREQUENCY_MS) .setRequiresDeviceIdle(true) - .setRequiresCharging(true); + .setRequiresCharging(true) + .setPersisted(true); + final JobInfo pending = jobScheduler.getPendingJob(R.integer.job_anomaly_config_update); - if (jobScheduler.schedule(jobBuilder.build()) != JobScheduler.RESULT_SUCCESS) { + // Don't schedule it if it already exists, to make sure it runs periodically even after + // reboot + if (pending == null && jobScheduler.schedule(jobBuilder.build()) + != JobScheduler.RESULT_SUCCESS) { Log.i(TAG, "Anomaly config update job service schedule failed."); } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobServiceTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobServiceTest.java index 98e4c57a683..a39276df44f 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobServiceTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyCleanupJobServiceTest.java @@ -18,10 +18,16 @@ package com.android.settings.fuelgauge.batterytip; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.robolectric.RuntimeEnvironment.application; import android.app.job.JobInfo; import android.app.job.JobScheduler; +import android.content.Context; import com.android.settings.R; import com.android.settings.testutils.SettingsRobolectricTestRunner; @@ -30,6 +36,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.MockitoAnnotations; +import org.robolectric.RuntimeEnvironment; import org.robolectric.Shadows; import org.robolectric.shadows.ShadowJobScheduler; @@ -38,17 +45,24 @@ import java.util.concurrent.TimeUnit; @RunWith(SettingsRobolectricTestRunner.class) public class AnomalyCleanupJobServiceTest { + private Context mContext; + private JobScheduler mJobScheduler; + @Before public void setUp() { MockitoAnnotations.initMocks(this); + + mContext = spy(RuntimeEnvironment.application); + mJobScheduler = spy(mContext.getSystemService(JobScheduler.class)); + when(mContext.getSystemService(JobScheduler.class)).thenReturn(mJobScheduler); } @Test public void testScheduleCleanUp() { - AnomalyCleanupJobService.scheduleCleanUp(application); + AnomalyCleanupJobService.scheduleCleanUp(mContext); ShadowJobScheduler shadowJobScheduler = - Shadows.shadowOf(application.getSystemService(JobScheduler.class)); + Shadows.shadowOf(mContext.getSystemService(JobScheduler.class)); List pendingJobs = shadowJobScheduler.getAllPendingJobs(); assertEquals(1, pendingJobs.size()); JobInfo pendingJob = pendingJobs.get(0); @@ -56,5 +70,14 @@ public class AnomalyCleanupJobServiceTest { assertThat(pendingJob.getIntervalMillis()).isEqualTo(TimeUnit.DAYS.toMillis(1)); assertThat(pendingJob.isRequireDeviceIdle()).isTrue(); assertThat(pendingJob.isRequireCharging()).isTrue(); + assertThat(pendingJob.isPersisted()).isTrue(); + } + + @Test + public void testScheduleCleanUp_invokeTwice_onlyScheduleOnce() { + AnomalyCleanupJobService.scheduleCleanUp(mContext); + AnomalyCleanupJobService.scheduleCleanUp(mContext); + + verify(mJobScheduler, times(1)).schedule(any()); } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobServiceTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobServiceTest.java index 5ff2c70288b..90af7b17c2a 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobServiceTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyConfigJobServiceTest.java @@ -25,7 +25,9 @@ import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.robolectric.RuntimeEnvironment.application; import android.app.StatsManager; @@ -43,6 +45,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.robolectric.RuntimeEnvironment; import org.robolectric.Shadows; import org.robolectric.shadows.ShadowJobScheduler; @@ -57,12 +60,18 @@ public class AnomalyConfigJobServiceTest { @Mock private StatsManager mStatsManager; + private Context mContext; + private JobScheduler mJobScheduler; private AnomalyConfigJobService mJobService; @Before public void setUp() { MockitoAnnotations.initMocks(this); + mContext = spy(RuntimeEnvironment.application); + mJobScheduler = spy(mContext.getSystemService(JobScheduler.class)); + when(mContext.getSystemService(JobScheduler.class)).thenReturn(mJobScheduler); + mJobService = spy(new AnomalyConfigJobService()); doReturn(application.getSharedPreferences(AnomalyConfigJobService.PREF_DB, Context.MODE_PRIVATE)).when(mJobService).getSharedPreferences(anyString(), @@ -71,11 +80,11 @@ public class AnomalyConfigJobServiceTest { } @Test - public void testScheduleCleanUp() { - AnomalyConfigJobService.scheduleConfigUpdate(application); + public void testScheduleConfigUpdate() { + AnomalyConfigJobService.scheduleConfigUpdate(mContext); ShadowJobScheduler shadowJobScheduler = - Shadows.shadowOf(application.getSystemService(JobScheduler.class)); + Shadows.shadowOf(mContext.getSystemService(JobScheduler.class)); List pendingJobs = shadowJobScheduler.getAllPendingJobs(); assertEquals(1, pendingJobs.size()); JobInfo pendingJob = pendingJobs.get(0); @@ -83,6 +92,15 @@ public class AnomalyConfigJobServiceTest { assertThat(pendingJob.getIntervalMillis()).isEqualTo(TimeUnit.DAYS.toMillis(1)); assertThat(pendingJob.isRequireDeviceIdle()).isTrue(); assertThat(pendingJob.isRequireCharging()).isTrue(); + assertThat(pendingJob.isPersisted()).isTrue(); + } + + @Test + public void testScheduleConfigUpdate_invokeTwice_onlyScheduleOnce() { + AnomalyConfigJobService.scheduleConfigUpdate(mContext); + AnomalyConfigJobService.scheduleConfigUpdate(mContext); + + verify(mJobScheduler, times(1)).schedule(any()); } @Test