[Offline pages] Updates to background scheduling to use BTS

chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/BackgroundSchedulerTest.java

Index: chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/BackgroundSchedulerTest.java
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/BackgroundSchedulerTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/BackgroundSchedulerTest.java
index 32b959e8b9865d5cc3acbcf5d786914efb7b3957..8046c72f2e534aa1ff9a53848e1c8327a3637cbb 100644
--- a/chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/BackgroundSchedulerTest.java
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/BackgroundSchedulerTest.java
@@ -5,98 +5,155 @@
 package org.chromium.chrome.browser.offlinepages;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-
-import android.content.Context;
-
-import com.google.android.gms.common.ConnectionResult;
-import com.google.android.gms.common.GoogleApiAvailability;
-import com.google.android.gms.gcm.GcmNetworkManager;
-import com.google.android.gms.gcm.Task;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.annotation.Config;
-import org.robolectric.internal.ShadowExtractor;
-import org.robolectric.shadows.gms.Shadows;
-import org.robolectric.shadows.gms.common.ShadowGoogleApiAvailability;
 
-import org.chromium.base.BaseChromiumApplication;
+import org.chromium.base.ContextUtils;
 import org.chromium.base.test.util.Feature;
+import org.chromium.components.background_task_scheduler.BackgroundTaskScheduler;
+import org.chromium.components.background_task_scheduler.BackgroundTaskSchedulerFactory;
+import org.chromium.components.background_task_scheduler.TaskIds;
+import org.chromium.components.background_task_scheduler.TaskInfo;
 import org.chromium.testing.local.LocalRobolectricTestRunner;
 
 /**
  * Unit tests for BackgroundScheduler.
  */
 @RunWith(LocalRobolectricTestRunner.class)
-@Config(manifest = Config.NONE, application = BaseChromiumApplication.class, sdk = 21,
-        shadows = {ShadowGcmNetworkManager.class, ShadowGoogleApiAvailability.class})
+@Config(manifest = Config.NONE)
 public class BackgroundSchedulerTest {
-    private Context mContext;
     private TriggerConditions mConditions1 = new TriggerConditions(
-            true /* power */, 10 /* battery percentage */, false /* unmetered */);
+            true /* power */, 10 /* battery percentage */, true /* requires unmetered */);
     private TriggerConditions mConditions2 = new TriggerConditions(
             false /* power */, 0 /* battery percentage */, false /* unmetered */);

[Pete Williamson, 2017/05/24 21:41:03]

Let's clarify this comment - does not require unmetered network

[fgorski, 2017/05/24 23:10:12]

Done.

-    private ShadowGcmNetworkManager mGcmNetworkManager;
+
+    @Mock
+    private BackgroundTaskScheduler mTaskScheduler;
+    @Captor
+    ArgumentCaptor<TaskInfo> mTaskInfo;
 
     @Before
     public void setUp() throws Exception {
-        Shadows.shadowOf(GoogleApiAvailability.getInstance())
-                .setIsGooglePlayServicesAvailable(ConnectionResult.SUCCESS);
-
-        mContext = RuntimeEnvironment.application;
-        mGcmNetworkManager = (ShadowGcmNetworkManager) ShadowExtractor.extract(
-                GcmNetworkManager.getInstance(mContext));
-        mGcmNetworkManager.clear();
+        MockitoAnnotations.initMocks(this);
+        ContextUtils.initApplicationContextForTests(RuntimeEnvironment.application);
+        BackgroundTaskSchedulerFactory.setSchedulerForTesting(mTaskScheduler);
+        doReturn(true)
+                .when(mTaskScheduler)
+                .schedule(eq(RuntimeEnvironment.application), mTaskInfo.capture());
     }
 
     @Test
     @Feature({"OfflinePages"})
-    public void testSchedule() {
-        assertNull(mGcmNetworkManager.getScheduledTask());
-        BackgroundScheduler.getInstance(mContext).schedule(mConditions1);
-        // Check with gcmNetworkManagerShadow that schedule got called.
-        assertNotNull(mGcmNetworkManager.getScheduledTask());
-
-        // Verify details of the scheduled task.
-        Task task = mGcmNetworkManager.getScheduledTask();
-        assertEquals(OfflinePageUtils.TASK_TAG, task.getTag());
-        long scheduledTimeMillis = TaskExtrasPacker.unpackTimeFromBundle(task.getExtras());
+    public void testScheduleUnmetered() {
+        BackgroundScheduler.getInstance().schedule(mConditions1);
+        verify(mTaskScheduler, times(1))
+                .schedule(eq(RuntimeEnvironment.application), eq(mTaskInfo.getValue()));
+
+        TaskInfo info = mTaskInfo.getValue();
+        assertEquals(TaskIds.OFFLINE_PAGES_BACKGROUND_JOB_ID, info.getTaskId());
+        assertEquals(OfflineBackgroundTask.class, info.getBackgroundTaskClass());
+        assertTrue(info.isPersisted());
+        assertTrue(info.shouldUpdateCurrent());
+        assertEquals(TaskInfo.NETWORK_TYPE_UNMETERED, info.getRequiredNetworkType());
+        assertTrue(info.requiresCharging());

[Pete Williamson, 2017/05/24 21:41:03]

It's a bit unfortunate that we use Charging in some places and Power elsewhere. 
Should we change them all to be the same?

[fgorski, 2017/05/24 23:10:12]

Good idea. Added a task for that.

+        assertFalse(info.isPeriodic());
+        assertEquals(BackgroundScheduler.NO_DELAY, info.getOneOffInfo().getWindowStartTimeMs());
+        assertEquals(BackgroundScheduler.ONE_WEEK_IN_MILLISECONDS,
+                info.getOneOffInfo().getWindowEndTimeMs());
+        assertTrue(info.getOneOffInfo().hasWindowStartTimeConstraint());
+
+        long scheduledTimeMillis = TaskExtrasPacker.unpackTimeFromBundle(info.getExtras());
         assertTrue(scheduledTimeMillis > 0L);
         assertEquals(
-                mConditions1, TaskExtrasPacker.unpackTriggerConditionsFromBundle(task.getExtras()));
+                mConditions1, TaskExtrasPacker.unpackTriggerConditionsFromBundle(info.getExtras()));
     }
 
     @Test
     @Feature({"OfflinePages"})
-    public void testCancel() {
-        assertNull(mGcmNetworkManager.getScheduledTask());
-        BackgroundScheduler.getInstance(mContext).schedule(mConditions1);
-        assertNotNull(mGcmNetworkManager.getScheduledTask());
+    public void testScheduleMetered() {

[Pete Williamson, 2017/05/24 21:41:03]

You're really testing metered and power here, not just metered (same with the
previous method which is testing unmetered and no power required).  Consider if
changing the test names to reflect that makes sense.

[fgorski, 2017/05/24 23:10:13]

Done.

+        BackgroundScheduler.getInstance().schedule(mConditions2);
+        verify(mTaskScheduler, times(1))
+                .schedule(eq(RuntimeEnvironment.application), eq(mTaskInfo.getValue()));
 
-        assertNull(mGcmNetworkManager.getCanceledTask());
-        BackgroundScheduler.getInstance(mContext).cancel();
-        assertNotNull(mGcmNetworkManager.getCanceledTask());
+        TaskInfo info = mTaskInfo.getValue();
+        assertEquals(TaskIds.OFFLINE_PAGES_BACKGROUND_JOB_ID, info.getTaskId());
+        assertEquals(OfflineBackgroundTask.class, info.getBackgroundTaskClass());
+        assertTrue(info.isPersisted());
+        assertTrue(info.shouldUpdateCurrent());
+        // These 2 lines are different.
+        assertEquals(TaskInfo.NETWORK_TYPE_ANY, info.getRequiredNetworkType());
+        assertFalse(info.requiresCharging());
+        assertFalse(info.isPeriodic());
+        assertEquals(BackgroundScheduler.NO_DELAY, info.getOneOffInfo().getWindowStartTimeMs());
+        assertEquals(BackgroundScheduler.ONE_WEEK_IN_MILLISECONDS,
+                info.getOneOffInfo().getWindowEndTimeMs());
+        assertTrue(info.getOneOffInfo().hasWindowStartTimeConstraint());
+
+        long scheduledTimeMillis = TaskExtrasPacker.unpackTimeFromBundle(info.getExtras());
+        assertTrue(scheduledTimeMillis > 0L);
+        // And the line below.
+        assertEquals(
+                mConditions2, TaskExtrasPacker.unpackTriggerConditionsFromBundle(info.getExtras()));
     }
 
     @Test
     @Feature({"OfflinePages"})
-    public void testReschedulOnUpgrade() {
-        assertNull(mGcmNetworkManager.getScheduledTask());
-        BackgroundScheduler.getInstance(mContext).rescheduleOfflinePagesTasksOnUpgrade();
-        // Check with gcmNetworkManagerShadow that schedule got called.
-        assertNotNull(mGcmNetworkManager.getScheduledTask());
-
-        // Verify details of the scheduled task.
-        Task task = mGcmNetworkManager.getScheduledTask();
-        assertEquals(OfflinePageUtils.TASK_TAG, task.getTag());
-        long scheduledTimeMillis = TaskExtrasPacker.unpackTimeFromBundle(task.getExtras());
+    public void testScheduleBackup() {

[Pete Williamson, 2017/05/24 21:41:03]

There is a lot of duplication in these tests - can they be refactored?

[fgorski, 2017/05/24 23:10:13]

Done.

[Pete Williamson, 2017/05/25 17:30:53]

The refactoring is good, thanks!

If you wanted, you might be able to take the refactoring one step further - if
you pass in as arguments to VerifyTaskInfoValues whether or not you require
charging or network, you could have VerifyTaskInfoValues check those too.

[fgorski, 2017/05/25 21:41:50]

I thought about it and decided not to, because I prefer to see the assert
failing inside of the test.
It is not about having as little code as possible, but rather to be able to
pin-point the failure.
Are you OK with that?

+        BackgroundScheduler.getInstance().scheduleBackup(
+                mConditions1, BackgroundScheduler.FIVE_MINUTES_IN_MILLISECONDS);
+        verify(mTaskScheduler, times(1))
+                .schedule(eq(RuntimeEnvironment.application), eq(mTaskInfo.getValue()));
+
+        TaskInfo info = mTaskInfo.getValue();
+        assertEquals(TaskIds.OFFLINE_PAGES_BACKGROUND_JOB_ID, info.getTaskId());
+        assertEquals(OfflineBackgroundTask.class, info.getBackgroundTaskClass());
+        assertTrue(info.isPersisted());
+        // This line is different.
+        assertFalse(info.shouldUpdateCurrent());
+        assertEquals(TaskInfo.NETWORK_TYPE_UNMETERED, info.getRequiredNetworkType());
+        assertTrue(info.requiresCharging());
+        assertFalse(info.isPeriodic());
+        assertEquals(BackgroundScheduler.FIVE_MINUTES_IN_MILLISECONDS,
+                info.getOneOffInfo().getWindowStartTimeMs());
+        assertEquals(BackgroundScheduler.ONE_WEEK_IN_MILLISECONDS,
+                info.getOneOffInfo().getWindowEndTimeMs());
+        assertTrue(info.getOneOffInfo().hasWindowStartTimeConstraint());
+
+        long scheduledTimeMillis = TaskExtrasPacker.unpackTimeFromBundle(info.getExtras());
         assertTrue(scheduledTimeMillis > 0L);
         assertEquals(
-                mConditions2, TaskExtrasPacker.unpackTriggerConditionsFromBundle(task.getExtras()));
+                mConditions1, TaskExtrasPacker.unpackTriggerConditionsFromBundle(info.getExtras()));
+    }
+
+    @Test
+    @Feature({"OfflinePages"})
+    public void testCancel() {
+        BackgroundScheduler.getInstance().schedule(mConditions1);
+        verify(mTaskScheduler, times(1))
+                .schedule(eq(RuntimeEnvironment.application), eq(mTaskInfo.getValue()));
+
+        doNothing()
+                .when(mTaskScheduler)
+                .cancel(eq(RuntimeEnvironment.application),
+                        eq(TaskIds.OFFLINE_PAGES_BACKGROUND_JOB_ID));
+        BackgroundScheduler.getInstance().cancel();
+        verify(mTaskScheduler, times(1))
+                .cancel(eq(RuntimeEnvironment.application),
+                        eq(TaskIds.OFFLINE_PAGES_BACKGROUND_JOB_ID));
     }
 }