Bug 1261713 - (Part 3) Refactor and add tests for extra experiment values support. r=sebastian
authorMargaret Leibovic <margaret.leibovic@gmail.com>
Sun, 03 Apr 2016 18:24:29 -0400
changeset 291793 27bd269e54937b99b0eef25ce849400a29c48fdb
parent 291792 ae1f2e223f22a0961192e4acf85280fdb445621e
child 291794 aedf35fb81d98ad4d53253f1abd680023f2848f1
push id74679
push userkwierso@gmail.com
push dateTue, 05 Apr 2016 23:39:26 +0000
treeherdermozilla-inbound@b70ae970d45d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian
bugs1261713
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1261713 - (Part 3) Refactor and add tests for extra experiment values support. r=sebastian MozReview-Commit-ID: A1enTZO7f9X
mobile/android/tests/background/junit4/src/com/keepsafe/switchboard/TestSwitchboard.java
mobile/android/thirdparty/com/keepsafe/switchboard/Switch.java
mobile/android/thirdparty/com/keepsafe/switchboard/SwitchBoard.java
--- a/mobile/android/tests/background/junit4/src/com/keepsafe/switchboard/TestSwitchboard.java
+++ b/mobile/android/tests/background/junit4/src/com/keepsafe/switchboard/TestSwitchboard.java
@@ -1,27 +1,29 @@
 package com.keepsafe.switchboard;
 
 import android.content.Context;
 
+import org.json.JSONException;
+import org.json.JSONObject;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mozilla.gecko.background.testhelpers.TestRunner;
 import org.robolectric.RuntimeEnvironment;
 
 import java.io.IOException;
 import java.util.UUID;
 
 import static org.junit.Assert.*;
 
 @RunWith(TestRunner.class)
 public class TestSwitchboard {
 
-    private static final String TEST_JSON = "{\"active-experiment\":{\"isActive\":true,\"values\":null},\"inactive-experiment\":{\"isActive\":false,\"values\":null}}";
+    private static final String TEST_JSON = "{\"active-experiment\":{\"isActive\":true,\"values\":{\"foo\": true}},\"inactive-experiment\":{\"isActive\":false,\"values\":null}}";
 
     @Before
     public void setUp() throws IOException {
         final Context c = RuntimeEnvironment.application;
 
         // Avoid hitting the network by setting a config directly.
         Preferences.setDynamicConfigJson(c, TEST_JSON);
     }
@@ -33,13 +35,24 @@ public class TestSwitchboard {
         final UUID uuid = df.getDeviceUuid();
         assertNotNull("UUID is not null", uuid);
         assertEquals("DeviceUuidFactory always returns the same UUID", df.getDeviceUuid(), uuid);
     }
 
     @Test
     public void testIsInExperiment() {
         final Context c = RuntimeEnvironment.application;
-        assertTrue("Active experiment is active", SwitchBoard.isInExperiment(c, "active-experiment"));
-        assertFalse("Inactive experiment is inactive", SwitchBoard.isInExperiment(c, "inactive-experiment"));
+        assertTrue("active-experiment is active", SwitchBoard.isInExperiment(c, "active-experiment"));
+        assertFalse("inactive-experiment is inactive", SwitchBoard.isInExperiment(c, "inactive-experiment"));
+    }
+
+    @Test
+    public void testExperimentValues() throws JSONException {
+        final Context c = RuntimeEnvironment.application;
+        assertTrue("active-experiment has values", SwitchBoard.hasExperimentValues(c, "active-experiment"));
+        assertFalse("inactive-experiment doesn't have values", SwitchBoard.hasExperimentValues(c, "inactive-experiment"));
+
+        final JSONObject values = SwitchBoard.getExperimentValuesFromJson(c, "active-experiment");
+        assertNotNull("active-experiment values are not null", values);
+        assertTrue("\"foo\" extra value is true", values.getBoolean("foo"));
     }
 
 }
--- a/mobile/android/thirdparty/com/keepsafe/switchboard/Switch.java
+++ b/mobile/android/thirdparty/com/keepsafe/switchboard/Switch.java
@@ -46,27 +46,27 @@ public class Switch {
      * Returns true if the experiment is active for this particular user.
      * @return Status of the experiment and false when experiment does not exist.
      */
     public boolean isActive() {
         return SwitchBoard.isInExperiment(context, experimentName);
     }
 
     /**
-     * Returns true if the experiment has aditional values.
+     * Returns true if the experiment has additional values.
      * @return true when values exist
      */
     public boolean hasValues() {
         return SwitchBoard.hasExperimentValues(context, experimentName);
     }
 
     /**
      * Gives back all the experiment values in a JSONObject. This function checks if
      * values exists. If no values exist, it returns null.
      * @return Values in JSONObject or null if non
      */
     public JSONObject getValues() {
         if(hasValues())
-            return SwitchBoard.getExperimentValueFromJson(context, experimentName);
+            return SwitchBoard.getExperimentValuesFromJson(context, experimentName);
         else
             return null;
     }
 }
--- a/mobile/android/thirdparty/com/keepsafe/switchboard/SwitchBoard.java
+++ b/mobile/android/thirdparty/com/keepsafe/switchboard/SwitchBoard.java
@@ -222,50 +222,41 @@ public class SwitchBoard {
         } catch (JSONException e) {
             // Something went wrong!
         }
 
         return returnList;
     }
 
     /**
-     * Checks if a certain experiment exists.
+     * Checks if a certain experiment has additional values.
      * @param c ApplicationContext
      * @param experimentName Name of the experiment
      * @return true when experiment exists
      */
     public static boolean hasExperimentValues(Context c, String experimentName) {
-        if(getExperimentValueFromJson(c, experimentName) == null)
-            return false;
-        else
-            return true;
+        return getExperimentValuesFromJson(c, experimentName) != null;
     }
 
     /**
-     * Returns the experiment value as a JSONObject. Depending on what experiment is has to be converted to the right type.
-     * Typcasting is by convention. You have to know what it's in there. Use <code>hasExperiment()</code>
-     * before this to avoid NullPointerExceptions.
-     * @param experimentName Name of the experiment to lookup
+     * Returns the experiment value as a JSONObject.
+     * @param experimentName Name of the experiment
      * @return Experiment value as String, null if experiment does not exist.
      */
-    public static JSONObject getExperimentValueFromJson(Context c, String experimentName) {
-        String config = Preferences.getDynamicConfigJson(c);
+    public static JSONObject getExperimentValuesFromJson(Context c, String experimentName) {
+        final String config = Preferences.getDynamicConfigJson(c);
 
-        if(config == null)
+        if (config == null) {
             return null;
+        }
 
         try {
-            JSONObject experiment = (JSONObject) new JSONObject(config).get(experimentName);
-            JSONObject values = experiment.getJSONObject(EXPERIMENT_VALUES);
-
-            return values;
-
+            final JSONObject experiment = new JSONObject(config).getJSONObject(experimentName);
+            return experiment.getJSONObject(EXPERIMENT_VALUES);
         } catch (JSONException e) {
-            Log.e(TAG, "Config: " + config);
-            e.printStackTrace();
             Log.e(TAG, "Could not create JSON object from config string", e);
         }
 
         return null;
     }
 
     /**
      * Returns a String containing the server response from a GET request