fix(testutils): never generate empty random user identities

android-core/src/test/java/com/mparticle/testutils/RandomUtilsTest.java

@@ -0,0 +1,65 @@
+package com.mparticle.testutils;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import com.mparticle.MParticle;
+
+import org.junit.Test;
+
+import java.util.Map;
+
+/**
+ * Guards the invariants that random identity generation must hold for the identity tests to be
+ * deterministic.
+ *
+ * <p>An empty identity map is not an innocuous edge case: {@code MParticleIdentityClientImpl.modify()}
+ * short-circuits and returns 200 without issuing an HTTP request when there are no identity
+ * changes, so a test that awaits the modify request hangs and then fails on an unrelated
+ * assertion. That was the cause of the intermittent
+ * {@code MParticleIdentityClientImplTest.testModifyMessage} failures.
+ */
+public class RandomUtilsTest {
+    private static final int ITERATIONS = 1000;
+
+    private final RandomUtils randomUtils = new RandomUtils();
+
+    @Test
+    public void testRandomUserIdentitiesNeverEmpty() {
+        for (int i = 0; i < ITERATIONS; i++) {
+            assertNonEmptyAndAliasFree(randomUtils.getRandomUserIdentities());
+        }
+    }
+
+    @Test
+    public void testBoundedRandomUserIdentitiesNeverEmpty() {
+        int poolSize = MParticle.IdentityType.values().length;
+        for (int max = 1; max <= poolSize + 1; max++) {
+            // Fewer iterations per bound: the pre-fix failure rate here was 1-in-22, so this is
+            // still overwhelmingly likely to catch a regression.
+            for (int i = 0; i < ITERATIONS / 10; i++) {
+                Map<MParticle.IdentityType, String> identities =
+                        randomUtils.getRandomUserIdentities(max);
+                assertNonEmptyAndAliasFree(identities);
+                assertTrue(
+                        "expected at most " + max + " identities but got " + identities.size(),
+                        identities.size() <= max);
+            }
+        }
+    }
+
+    @Test
+    public void testMockRandomUserIdentitiesNeverEmpty() {
+        for (int i = 0; i < ITERATIONS; i++) {
+            assertNonEmptyAndAliasFree(
+                    com.mparticle.mock.utils.RandomUtils.getInstance().getRandomUserIdentities());
+        }
+    }
+
+    private void assertNonEmptyAndAliasFree(Map<MParticle.IdentityType, String> identities) {
+        assertFalse("random user identities must never be empty", identities.isEmpty());
+        assertFalse(
+                "Alias is not a settable user identity and must never be generated",
+                identities.containsKey(MParticle.IdentityType.Alias));
+    }
+}

testutils/src/main/java/com/mparticle/mock/utils/RandomUtils.java

@@ -7,6 +7,8 @@
 import org.junit.Test;
 
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -24,23 +26,41 @@ public class RandomUtils {
 
     private static RandomUtils instance;
 
+    /**
+     * Every IdentityType that can legitimately appear in a user identity map. Alias is excluded
+     * from the pool rather than removed after the draw -- removing it afterwards can leave the
+     * map empty, which silently changes the meaning of any request built from it.
+     */
+    private static final List<MParticle.IdentityType> ASSIGNABLE_IDENTITY_TYPES =
+            assignableIdentityTypes();
+
+    private static List<MParticle.IdentityType> assignableIdentityTypes() {
+        List<MParticle.IdentityType> types =
+                new ArrayList<MParticle.IdentityType>(Arrays.asList(MParticle.IdentityType.values()));
+        types.remove(MParticle.IdentityType.Alias);
+        return Collections.unmodifiableList(types);
+    }
+
     public static RandomUtils getInstance() {
         if (instance == null) {
             instance = new RandomUtils();
         }
         return instance;
     }
 
+    /**
+     * @return between 1 and the number of assignable IdentityTypes distinct random user
+     * identities. Never empty.
+     */
     public Map<MParticle.IdentityType, String> getRandomUserIdentities() {
-        Map<MParticle.IdentityType, String> randomIdentities = new HashMap<MParticle.IdentityType, String>();
+        int poolSize = ASSIGNABLE_IDENTITY_TYPES.size();
+        int numIdentities = randomInt(1, poolSize + 1);
 
-        int identityTypeLength = MParticle.IdentityType.values().length;
-        int numIdentities = randomInt(1, identityTypeLength);
-        Set<Integer> identityIndices = randomIntSet(0, identityTypeLength, numIdentities);
+        Map<MParticle.IdentityType, String> randomIdentities = new HashMap<MParticle.IdentityType, String>();
+        Set<Integer> identityIndices = randomIntSet(0, poolSize, numIdentities);
         for (Integer identityIndex : identityIndices) {
-            randomIdentities.put(MParticle.IdentityType.values()[identityIndex], getAlphaNumericString(randomInt(1, 55)));
+            randomIdentities.put(ASSIGNABLE_IDENTITY_TYPES.get(identityIndex), getAlphaNumericString(randomInt(1, 55)));
         }
-        randomIdentities.remove(MParticle.IdentityType.Alias);
         return randomIdentities;
     }
 

testutils/src/main/java/com/mparticle/testutils/RandomUtils.java

@@ -2,7 +2,11 @@
 
 import com.mparticle.MParticle;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Random;
 import java.util.Set;
@@ -16,20 +20,39 @@ public class RandomUtils {
     private static final String sNumbers = "0123456789";
     private static final String sCharacters = " ,.";
 
+    /**
+     * Every IdentityType that can legitimately appear in a user identity map. Alias is excluded
+     * from the pool rather than removed after the draw -- removing it afterwards can leave the
+     * map empty, which silently changes the meaning of any request built from it.
+     */
+    private static final List<MParticle.IdentityType> ASSIGNABLE_IDENTITY_TYPES =
+            assignableIdentityTypes();
+
+    private static List<MParticle.IdentityType> assignableIdentityTypes() {
+        List<MParticle.IdentityType> types =
+                new ArrayList<MParticle.IdentityType>(Arrays.asList(MParticle.IdentityType.values()));
+        types.remove(MParticle.IdentityType.Alias);
+        return Collections.unmodifiableList(types);
+    }
+
     public Map<MParticle.IdentityType, String> getRandomUserIdentities() {
         return getRandomUserIdentities(null);
     }
 
+    /**
+     * @param max the most identities to return, or null for no limit
+     * @return between 1 and {@code max} distinct random user identities. Never empty.
+     */
     public Map<MParticle.IdentityType, String> getRandomUserIdentities(Integer max) {
-        Map<MParticle.IdentityType, String> randomIdentities = new HashMap<MParticle.IdentityType, String>();
+        int poolSize = ASSIGNABLE_IDENTITY_TYPES.size();
+        int upperBound = (max != null && max < poolSize) ? Math.max(1, max) : poolSize;
+        int numIdentities = randomInt(1, upperBound + 1);
 
-        int identityTypeLength = MParticle.IdentityType.values().length;
-        int numIdentities = randomInt(1, (max != null && max < identityTypeLength) ? max : identityTypeLength);
-        Set<Integer> identityIndices = randomIntSet(0, identityTypeLength, numIdentities);
+        Map<MParticle.IdentityType, String> randomIdentities = new HashMap<MParticle.IdentityType, String>();
+        Set<Integer> identityIndices = randomIntSet(0, poolSize, numIdentities);
         for (Integer identityIndex : identityIndices) {
-            randomIdentities.put(MParticle.IdentityType.values()[identityIndex], getAlphaNumericString(randomInt(1, 55)));
+            randomIdentities.put(ASSIGNABLE_IDENTITY_TYPES.get(identityIndex), getAlphaNumericString(randomInt(1, 55)));
         }
-        randomIdentities.remove(MParticle.IdentityType.Alias);
         return randomIdentities;
     }