From 7e1eae668131ce0bd81b3587ca2e6b6a40359f74 Mon Sep 17 00:00:00 2001 From: Bartlomiej Niechwiej Date: Thu, 11 Nov 2010 10:55:15 -0800 Subject: [PATCH] Addressed Sandor's comments and added more unit tests and fixed 2 NPEs. --- .../services/PeriodicTaskExecuter.java | 5 ++ .../apps/mytracks/services/SplitManager.java | 11 ++++ .../services/TrackRecordingServiceTest.java | 57 +++++++++++++++++-- 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/MyTracks/src/com/google/android/apps/mytracks/services/PeriodicTaskExecuter.java b/MyTracks/src/com/google/android/apps/mytracks/services/PeriodicTaskExecuter.java index d4e61c7a3..737d051d7 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/services/PeriodicTaskExecuter.java +++ b/MyTracks/src/com/google/android/apps/mytracks/services/PeriodicTaskExecuter.java @@ -52,6 +52,11 @@ public class PeriodicTaskExecuter { * @param interval The interval in milliseconds */ public void scheduleTask(long interval) { + // TODO: Decouple service from this class once and forever. + if (!service.isRecording()) { + return; + } + timer.cancel(); timer.purge(); timer = new Timer(); diff --git a/MyTracks/src/com/google/android/apps/mytracks/services/SplitManager.java b/MyTracks/src/com/google/android/apps/mytracks/services/SplitManager.java index 857d48316..39f4c61ee 100644 --- a/MyTracks/src/com/google/android/apps/mytracks/services/SplitManager.java +++ b/MyTracks/src/com/google/android/apps/mytracks/services/SplitManager.java @@ -73,6 +73,11 @@ public class SplitManager { * Calculates the next distance that a split should be inserted at. */ public void calculateNextSplit() { + // TODO: Decouple service from this class once and forever. + if (!service.isRecording()) { + return; + } + if (splitFrequency >= 0) { nextSplitDistance = Double.MAX_VALUE; Log.d(MyTracksConstants.TAG, @@ -121,6 +126,12 @@ public class SplitManager { */ public void setSplitFrequency(int splitFrequency) { this.splitFrequency = splitFrequency; + + // TODO: Decouple service from this class once and forever. + if (!service.isRecording()) { + return; + } + if (splitFrequency < 1) { if (splitExecuter != null) { splitExecuter.shutdown(); diff --git a/MyTracksTest/src/com/google/android/apps/mytracks/services/TrackRecordingServiceTest.java b/MyTracksTest/src/com/google/android/apps/mytracks/services/TrackRecordingServiceTest.java index f0466a1f9..1fa7a581d 100644 --- a/MyTracksTest/src/com/google/android/apps/mytracks/services/TrackRecordingServiceTest.java +++ b/MyTracksTest/src/com/google/android/apps/mytracks/services/TrackRecordingServiceTest.java @@ -46,6 +46,11 @@ import java.util.List; * Tests for the MyTracks track recording service. * * @author Bartlomiej Niechwiej + * + * TODO: The original class, ServiceTestCase, has a few limitations, e.g. + * it's not possible to properly shutdown the service, unless tearDown() + * is called, which prevents from testing multiple scenarios in a single + * test (see runFunctionTest for more details). */ public class TrackRecordingServiceTest extends ServiceTestCase { @@ -554,13 +559,50 @@ public class TrackRecordingServiceTest public void testWithProperties_defaultMinRecordingDist() throws Exception { functionalTest(R.string.min_recording_distance_key, 2); } + + @MediumTest + public void testWithProperties_noSignalSamplingFreq() throws Exception { + functionalTest(R.string.signal_sampling_frequency_key, (Object) null); + } + + @MediumTest + public void testWithProperties_defaultSignalSamplingFreq() throws Exception { + functionalTest(R.string.signal_sampling_frequency_key, 1); + } + + @MediumTest + public void testWithProperties_noSplitFreq() throws Exception { + functionalTest(R.string.split_frequency_key, (Object) null); + } + + @MediumTest + public void testWithProperties_defaultSplitFreqByDist() throws Exception { + functionalTest(R.string.split_frequency_key, 5); + } + + @MediumTest + public void testWithProperties_defaultSplitFreqByTime() throws Exception { + functionalTest(R.string.split_frequency_key, -2); + } + + @MediumTest + public void testWithProperties_noMetricUnits() throws Exception { + functionalTest(R.string.metric_units_key, (Object) null); + } + + @MediumTest + public void testWithProperties_metricUnitsEnabled() throws Exception { + functionalTest(R.string.metric_units_key, true); + } + + @MediumTest + public void testWithProperties_metricUnitsDisabled() throws Exception { + functionalTest(R.string.metric_units_key, false); + } // TODO: Add the following tests: - // R.string.metric_units_key // R.string.min_recording_interval_key // R.string.min_required_accuracy_key - // R.string.signal_sampling_frequency_key - // R.string.split_frequency_key private ITrackRecordingService bindAndGetService(Intent intent) { ITrackRecordingService service = ITrackRecordingService.Stub.asInterface( @@ -607,7 +649,10 @@ public class TrackRecordingServiceTest editor.putLong(context.getString(R.string.recording_track_key), id); editor.commit(); } - + + // TODO: We support multiple values for readability, however this test's + // base class doesn't properly shutdown the service, so it's not possible + // to pass more than 1 value at a time. private void functionalTest(int resourceId, Object ...values) throws Exception { final String key = context.getString(resourceId); @@ -621,6 +666,8 @@ public class TrackRecordingServiceTest editor.putLong(key, (Long) value); } else if (value instanceof Integer) { editor.putInt(key, (Integer) value); + } else if (value instanceof Boolean) { + editor.putBoolean(key, (Boolean) value); } else if (value == null) { // Do nothing, as clear above has already removed this property. } @@ -645,6 +692,8 @@ public class TrackRecordingServiceTest context.getString(R.string.recording_track_key), -1)); assertEquals(id, service.getRecordingTrackId()); + // TODO: Add a few locations, insert markers, etc. + // Stop the track. Validate if it has correct data. service.endCurrentTrack(); assertFalse(service.isRecording());