More unit tests for TracksRecordingService and fix a few bugs. Made the service less forgiving rather than silently ignoring invalid state.

MyTracks/src/com/google/android/apps/mytracks/services/PreferenceManager.java

@@ -20,6 +20,7 @@ import com.google.android.apps.mytracks.MyTracksSettings;
 import com.google.android.maps.mytracks.R;
 
 import android.content.SharedPreferences;
+import android.content.SharedPreferences.Editor;
 import android.util.Log;
 
 /**
@@ -40,8 +41,17 @@ public class PreferenceManager {
   private final String signalSamplingFrequencyKey;
   private final String splitFrequencyKey;
 
+  private final SharedPreferences sharedPreferences;
+
   public PreferenceManager(TrackRecordingService service) {
     this.service = service;
+    this.sharedPreferences =
+        service.getSharedPreferences(MyTracksSettings.SETTINGS_NAME, 0);
+    if (sharedPreferences == null) {
+      Log.w(MyTracksConstants.TAG,
+          "TrackRecordingService: Couldn't get shared preferences.");
+      throw new IllegalStateException("Couldn't get shared preferences");
+    }
     
     announcementFrequencyKey =
         service.getString(R.string.announcement_frequency_key);
@@ -57,12 +67,12 @@ public class PreferenceManager {
         service.getString(R.string.min_recording_interval_key);
     minRequiredAccuracyKey =
         service.getString(R.string.min_required_accuracy_key);
-    splitFrequencyKey =
-        service.getString(R.string.split_frequency_key);
-    signalSamplingFrequencyKey =
-        service.getString(R.string.signal_sampling_frequency_key);
     recordingTrackKey =
         service.getString(R.string.recording_track_key);
+    signalSamplingFrequencyKey =
+        service.getString(R.string.signal_sampling_frequency_key);
+    splitFrequencyKey =
+        service.getString(R.string.split_frequency_key);
   }
 
   /**
@@ -72,14 +82,6 @@ public class PreferenceManager {
    * @param key the key that changed (may be null to update all preferences)
    */
   public void onSharedPreferenceChanged(String key) {
-    SharedPreferences sharedPreferences =
-        service.getSharedPreferences(MyTracksSettings.SETTINGS_NAME, 0);
-    if (sharedPreferences == null) {
-      Log.w(MyTracksConstants.TAG,
-          "TrackRecordingService: Couldn't get shared preferences.");
-      return;
-    }
-
     if (key == null || key.equals(minRecordingDistanceKey)) {
       service.setMinRecordingDistance(
           sharedPreferences.getInt(
@@ -156,4 +158,16 @@ public class PreferenceManager {
           sharedPreferences.getBoolean(metricUnitsKey, true));
     }
   }
+  
+  public void setAutoResumeTrackCurrentRetry(int retryAttempts) {
+    SharedPreferences.Editor editor = sharedPreferences.edit();
+    editor.putInt(autoResumeTrackTimeoutKey, retryAttempts);
+    editor.commit();
+  }
+
+  public void setRecordingTrack(long id) {
+    Editor editor = sharedPreferences.edit();
+    editor.putLong(recordingTrackKey, id);
+    editor.commit();
+  }
 }

MyTracks/src/com/google/android/apps/mytracks/services/TrackRecordingService.java

@@ -67,7 +67,7 @@ public class TrackRecordingService extends Service implements LocationListener {
   private static final String STATISTICS_ICON_URL =
       "http://maps.google.com/mapfiles/ms/micons/ylw-pushpin.png";
 
-  private static final int MAX_AUTO_RESUME_TRACK_RETRY_ATTEMPTS = 3;
+  static final int MAX_AUTO_RESUME_TRACK_RETRY_ATTEMPTS = 3;
   
   private NotificationManager notificationManager;
   private LocationManager locationManager;
@@ -382,6 +382,7 @@ public class TrackRecordingService extends Service implements LocationListener {
   }
   
   private Track getRecordingTrack() {
+    Log.d(MyTracksConstants.TAG, "Recording track ID: " + recordingTrackId);
     if (recordingTrackId < 0) {
       return null;
     }
@@ -390,6 +391,9 @@ public class TrackRecordingService extends Service implements LocationListener {
   }
 
   private void restoreStats(Track track) {
+    Log.d(MyTracksConstants.TAG,
+        "Restoring stats of track with ID: " + track.getId());
+    
     TripStatistics stats = track.getStatistics();
     statsBuilder = new TripStatisticsBuilder();
     statsBuilder.resumeAt(stats.getStartTime());
@@ -739,8 +743,9 @@ public class TrackRecordingService extends Service implements LocationListener {
       // Make sure that the current track exists and is fresh enough.
       if (recordingTrack == null || !shouldResumeTrack(recordingTrack)) {
         Log.i(MyTracksConstants.TAG,
-            "TrackRecordingService: Not resuming because the previous track "
+            "TrackRecordingService: Not resuming, because the previous track ("
-            + "doesn't exist or is too old");
+            + recordingTrack + ") doesn't exist or is too old");
+        isRecording = false;
         stopSelfResult(startId);
         return;
       }
@@ -753,11 +758,7 @@ public class TrackRecordingService extends Service implements LocationListener {
       SharedPreferences sharedPreferences, int retryAttempts) {
     Log.d(MyTracksConstants.TAG,
         "Updating auto-resume retry attempts to: " + retryAttempts);
-    
+    prefManager.setAutoResumeTrackCurrentRetry(retryAttempts);
-    SharedPreferences.Editor editor = sharedPreferences.edit();
-    editor.putInt(
-        getString(R.string.auto_resume_track_current_retry_key), retryAttempts);
-    editor.commit();
   }
   
   private boolean shouldResumeTrack(Track track) {
@@ -908,6 +909,10 @@ public class TrackRecordingService extends Service implements LocationListener {
         @Override
         public void endCurrentTrack() {
           Log.d(MyTracksConstants.TAG, "TrackRecordingService.endCurrentTrack");
+          if (recordingTrackId == -1 || !isRecording) {
+            throw new IllegalStateException("No recording track in progress!");
+          }
+         
           isRecording = false;
           Track recordingTrack = providerUtils.getTrack(recordingTrackId);
           if (recordingTrack != null) {
@@ -928,6 +933,7 @@ public class TrackRecordingService extends Service implements LocationListener {
           }
           showNotification();
           recordingTrackId = -1;
+          prefManager.setRecordingTrack(recordingTrackId);
         }
 
         @Override
@@ -951,6 +957,10 @@ public class TrackRecordingService extends Service implements LocationListener {
 
   public long startNewTrack() {
     Log.d(MyTracksConstants.TAG, "TrackRecordingService.startNewTrack");
+    if (recordingTrackId != -1 || isRecording) {
+      throw new IllegalStateException("A track is already in progress!");
+    }
+    
     Track track = new Track();
     TripStatistics trackStats = track.getStatistics();
     track.setName("new");
@@ -973,10 +983,13 @@ public class TrackRecordingService extends Service implements LocationListener {
     registerLocationListener();
     splitManager.restore();
     signalManager.restore();
+
     // Reset the number of auto-resume retries.
-    SharedPreferences sharedPreferences =
+    setAutoResumeTrackRetries(
-        getSharedPreferences(MyTracksSettings.SETTINGS_NAME, 0); 
+        getSharedPreferences(MyTracksSettings.SETTINGS_NAME, 0), 0);
-    setAutoResumeTrackRetries(sharedPreferences, 0);
+    // Persist the current recording track.
+    prefManager.setRecordingTrack(recordingTrackId);
+    
     return recordingTrackId;
   }
 

MyTracksTest/src/com/google/android/apps/mytracks/services/TrackRecordingServiceTest.java

@@ -15,12 +15,31 @@
  */
 package com.google.android.apps.mytracks.services;
 
+import static com.google.android.apps.mytracks.MyTracksConstants.RESUME_TRACK_EXTRA_NAME;
+
+import com.google.android.apps.mytracks.MyTracksSettings;
+import com.google.android.apps.mytracks.content.MyTracksProvider;
+import com.google.android.apps.mytracks.content.MyTracksProviderUtils;
+import com.google.android.apps.mytracks.content.Track;
+import com.google.android.apps.mytracks.stats.TripStatistics;
+import com.google.android.maps.mytracks.R;
+
+import android.content.ContentResolver;
+import android.content.Context;
+import android.content.ContextWrapper;
 import android.content.Intent;
+import android.content.SharedPreferences;
+import android.content.SharedPreferences.Editor;
 import android.os.IBinder;
+import android.os.RemoteException;
+import android.test.RenamingDelegatingContext;
 import android.test.ServiceTestCase;
+import android.test.mock.MockContentResolver;
 import android.test.suitebuilder.annotation.MediumTest;
 import android.test.suitebuilder.annotation.SmallTest;
 
+import java.util.List;
+
 /**
  * Tests for the MyTracks track recording service.
  * 
@@ -29,23 +48,257 @@ import android.test.suitebuilder.annotation.SmallTest;
 public class TrackRecordingServiceTest
     extends ServiceTestCase<TrackRecordingService> {
 
+  private Context context;
+  private MyTracksProviderUtils providerUtils;
+  private SharedPreferences sharedPreferences;
+  
   public TrackRecordingServiceTest() {
     super(TrackRecordingService.class);
   }
 
+  private static class MockContext extends ContextWrapper {
+    private final ContentResolver contentResolver;
+    
+    public MockContext(ContentResolver contentResolver, Context base) {
+      super(base);
+      this.contentResolver = contentResolver;
+    }
+
+    @Override
+    public ContentResolver getContentResolver() {
+      return contentResolver;
+    }
+  }
+  
+  @Override
+  protected void setUp() throws Exception {
+    super.setUp();
+    
+    MockContentResolver mockContentResolver = new MockContentResolver();
+    RenamingDelegatingContext targetContext = new RenamingDelegatingContext(
+        getContext(), getContext(), "test.");
+    context = new MockContext(mockContentResolver, targetContext);
+    MyTracksProvider provider = new MyTracksProvider();
+    provider.attachInfo(context, null);
+    mockContentResolver.addProvider(MyTracksProviderUtils.AUTHORITY, provider);
+    setContext(context);
+    
+    providerUtils = MyTracksProviderUtils.Factory.get(context);
+    
+    sharedPreferences = context.getSharedPreferences(
+        MyTracksSettings.SETTINGS_NAME, 0);
+    // Disable auto resume by default.
+    updateAutoResumePrefs(0, -1);
+    // No recording track.
+    Editor editor = sharedPreferences.edit();
+    editor.putLong(context.getString(R.string.recording_track_key), -1);
+    editor.commit();
+  }
+
   @SmallTest
   public void testStartable() {
-    Intent startIntent = new Intent();
+    startService(createStartIntent());
-    startIntent.setClass(getContext(), TrackRecordingService.class);
-    startService(startIntent);
     assertNotNull(getService());
   }
 
   @MediumTest
   public void testBindable() {
-    Intent startIntent = new Intent();
+    IBinder service = bindService(createStartIntent());
-    startIntent.setClass(getContext(), TrackRecordingService.class);
-    IBinder service = bindService(startIntent);
     assertNotNull(service);
   }
+  
+  @MediumTest
+  public void testResumeAfterReboot_shouldResume() throws Exception {
+    // Insert a dummy track and mark it as recording track.
+    createDummyTrack(123, System.currentTimeMillis(), true);
+
+    // Clear the number of attempts and set the timeout to 10 min.
+    updateAutoResumePrefs(0, 10);
+
+    // Resume the service.  It should resume recording of the previous track.
+    Intent startIntent = createStartIntent();
+    startIntent.putExtra(RESUME_TRACK_EXTRA_NAME, true);
+    startService(startIntent);
+    assertNotNull(getService());
+
+    assertTrue(getService().isRecording());
+  }
+
+  @MediumTest
+  public void testResumeAfterReboot_noRecordingTrack() throws Exception {
+    // Insert a dummy track and mark it as recording track.
+    createDummyTrack(123, System.currentTimeMillis(), false);
+
+    // Clear the number of attempts and set the timeout to 10 min.
+    updateAutoResumePrefs(0, 10);
+
+    // Resume the service.  It should resume recording of the previous track.
+    Intent startIntent = createStartIntent();
+    startIntent.putExtra(RESUME_TRACK_EXTRA_NAME, true);
+    startService(startIntent);
+    assertNotNull(getService());
+    
+    assertFalse(getService().isRecording());
+  }
+  
+  @MediumTest
+  public void testResumeAfterReboot_expiredTrack() throws Exception {
+    // Insert a dummy track last updated 20 min ago.
+    createDummyTrack(123, System.currentTimeMillis() - 20 * 60 * 1000, true);
+
+    // Clear the number of attempts and set the timeout to 10 min.
+    updateAutoResumePrefs(0, 10);
+
+    // Resume the service.  It should resume recording of the previous track.
+    Intent startIntent = createStartIntent();
+    startIntent.putExtra(RESUME_TRACK_EXTRA_NAME, true);
+    startService(startIntent);
+    assertNotNull(getService());
+    
+    assertFalse(getService().isRecording());
+  }
+
+  @MediumTest
+  public void testResumeAfterReboot_tooManyAttempts() throws Exception {
+    // Insert a dummy track.
+    createDummyTrack(123, System.currentTimeMillis(), true);
+
+    // Set the number of attempts to max.
+    updateAutoResumePrefs(
+        TrackRecordingService.MAX_AUTO_RESUME_TRACK_RETRY_ATTEMPTS, 10);
+
+    // Resume the service.  It should resume recording of the previous track.
+    Intent startIntent = createStartIntent();
+    startIntent.putExtra(RESUME_TRACK_EXTRA_NAME, true);
+    startService(startIntent);
+    assertNotNull(getService());
+    
+    assertFalse(getService().isRecording());
+  }
+
+  @MediumTest
+  public void testRecording_noTracks() throws Exception {
+    List<Track> tracks = providerUtils.getAllTracks();
+    assertTrue(tracks.isEmpty());
+    
+    ITrackRecordingService service = startAndGetService(createStartIntent());
+    // Test if we start in no-recording mode by default. 
+    assertFalse(service.isRecording());
+  }
+  
+  @MediumTest
+  public void testRecording_oldTracks() throws Exception {
+    createDummyTrack(123, -1, false);
+    
+    ITrackRecordingService service = startAndGetService(createStartIntent());
+    assertFalse(service.isRecording());
+  }
+
+  @MediumTest
+  public void testStartNewTrack_noRecording() throws Exception {
+    List<Track> tracks = providerUtils.getAllTracks();
+    assertTrue(tracks.isEmpty());
+    
+    ITrackRecordingService service = startAndGetService(createStartIntent());
+    assertFalse(service.isRecording());
+    
+    long id = service.startNewTrack();
+    assertTrue(id >= 0);
+    assertTrue(service.isRecording());
+    Track track = providerUtils.getTrack(id);
+    assertNotNull(track);
+    assertEquals(id, track.getId());
+    assertEquals(id, sharedPreferences.getLong(
+        context.getString(R.string.recording_track_key), -1));
+  }
+  
+  @MediumTest
+  public void testStartNewTrack_alreadyRecording() throws Exception {
+    createDummyTrack(123, -1, true);
+    
+    ITrackRecordingService service = startAndGetService(createStartIntent());
+    assertTrue(service.isRecording());
+    
+    try {
+      service.startNewTrack();
+      fail("Expecting RemoteException");
+    } catch (IllegalStateException e) {
+      // Expected.
+    }
+    assertEquals(123, sharedPreferences.getLong(
+        context.getString(R.string.recording_track_key), 0));
+  }  
+  
+  @MediumTest
+  public void testEndCurrentTrack_alreadyRecording() throws Exception {
+    createDummyTrack(123, -1, true);
+    
+    ITrackRecordingService service = startAndGetService(createStartIntent());
+    assertTrue(service.isRecording());
+
+    // End the current track.
+    service.endCurrentTrack();
+    assertFalse(service.isRecording());
+    assertEquals(-1, sharedPreferences.getLong(
+        context.getString(R.string.recording_track_key), 0));
+  }
+  
+  @MediumTest
+  public void testEndCurrentTrack_noRecording() throws Exception {
+    ITrackRecordingService service = startAndGetService(createStartIntent());
+    assertFalse(service.isRecording());
+
+    // End the current track.
+    try {
+      service.endCurrentTrack();
+      fail("Expecting RemoteException");
+    } catch (IllegalStateException e) {
+      // Expected.
+    }
+    assertEquals(-1, sharedPreferences.getLong(
+        context.getString(R.string.recording_track_key), 0));
+  }
+  
+  private ITrackRecordingService startAndGetService(Intent intent) {
+    ITrackRecordingService service = ITrackRecordingService.Stub.asInterface(
+        bindService(intent));
+    assertNotNull(service);
+    return service;
+  }
+
+  private Track createDummyTrack(long id, long stopTime, boolean isRecording) {
+    Track dummyTrack = new Track();
+    dummyTrack.setId(id);
+    dummyTrack.setName("Dummy Track");
+    TripStatistics tripStatistics = new TripStatistics();
+    tripStatistics.setStopTime(stopTime);
+    dummyTrack.setStatistics(tripStatistics);
+    addTrack(dummyTrack, isRecording);
+    return dummyTrack;
+  }
+  
+  private void updateAutoResumePrefs(int attempts, int timeoutMins) {
+    Editor editor = sharedPreferences.edit();
+    editor.putInt(context.getString(
+    R.string.auto_resume_track_current_retry_key), attempts);
+    editor.putInt(context.getString(
+        R.string.auto_resume_track_timeout_key), timeoutMins);
+    editor.commit();
+  }
+  
+  private Intent createStartIntent() {
+    Intent startIntent = new Intent();
+    startIntent.setClass(context, TrackRecordingService.class);
+    return startIntent;
+  }
+  
+  private void addTrack(Track track, boolean isRecording) {
+    assertTrue(track.getId() >= 0);
+    providerUtils.insertTrack(track);
+    assertEquals(track.getId(), providerUtils.getTrack(track.getId()).getId());
+    Editor editor = sharedPreferences.edit();
+    editor.putLong(context.getString(R.string.recording_track_key),
+        isRecording ? track.getId() : -1);
+    editor.commit();
+  }
 }