From f40c0a0cfc0fa11ccbd43d70175a72da184b76ff Mon Sep 17 00:00:00 2001 From: James Newman Date: Mon, 3 Aug 2026 13:14:06 -0400 Subject: [PATCH] fix(testutils): never generate empty random user identities getRandomUserIdentities() drew indices from the full IdentityType enum and then removed Alias afterwards, so a draw of exactly one index that landed on Alias returned an empty map (P ~ 1/462 for the no-arg overload, and 1/22 for getRandomUserIdentities(2), where the bound forced a single draw). An empty identity map is not benign. MParticleIdentityClientImpl.modify() short-circuits and returns 200 without issuing an HTTP request when there are no identity changes, so a test awaiting the modify request never sees it. That is the cause of the intermittent MParticleIdentityClientImplTest.testModifyMessage failures, which accounted for 3 of the 21 instrumented-test failures across the last 150 pull-request workflow runs. Exclude Alias from the candidate pool instead of removing it after the draw, so the result can never be empty. Also makes the max bound inclusive, which fixes an ArithmeticException (divide by zero) on getRandomUserIdentities(1). Adds RandomUtilsTest in android-core/src/test, which is where it will actually run -- the testutils module has no test source set, so the pre-existing @Test in mock/utils/RandomUtils is never discovered by any test task. Co-Authored-By: Claude Opus 5 --- .../mparticle/testutils/RandomUtilsTest.java | 65 +++++++++++++++++++ .../com/mparticle/mock/utils/RandomUtils.java | 32 +++++++-- .../com/mparticle/testutils/RandomUtils.java | 35 ++++++++-- 3 files changed, 120 insertions(+), 12 deletions(-) create mode 100644 android-core/src/test/java/com/mparticle/testutils/RandomUtilsTest.java diff --git a/android-core/src/test/java/com/mparticle/testutils/RandomUtilsTest.java b/android-core/src/test/java/com/mparticle/testutils/RandomUtilsTest.java new file mode 100644 index 000000000..cf5456259 --- /dev/null +++ b/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. + * + *

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 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 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)); + } +} diff --git a/testutils/src/main/java/com/mparticle/mock/utils/RandomUtils.java b/testutils/src/main/java/com/mparticle/mock/utils/RandomUtils.java index 6e3e2b439..ca396930d 100644 --- a/testutils/src/main/java/com/mparticle/mock/utils/RandomUtils.java +++ b/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,6 +26,21 @@ 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 ASSIGNABLE_IDENTITY_TYPES = + assignableIdentityTypes(); + + private static List assignableIdentityTypes() { + List types = + new ArrayList(Arrays.asList(MParticle.IdentityType.values())); + types.remove(MParticle.IdentityType.Alias); + return Collections.unmodifiableList(types); + } + public static RandomUtils getInstance() { if (instance == null) { instance = new RandomUtils(); @@ -31,16 +48,19 @@ public static RandomUtils getInstance() { return instance; } + /** + * @return between 1 and the number of assignable IdentityTypes distinct random user + * identities. Never empty. + */ public Map getRandomUserIdentities() { - Map randomIdentities = new HashMap(); + int poolSize = ASSIGNABLE_IDENTITY_TYPES.size(); + int numIdentities = randomInt(1, poolSize + 1); - int identityTypeLength = MParticle.IdentityType.values().length; - int numIdentities = randomInt(1, identityTypeLength); - Set identityIndices = randomIntSet(0, identityTypeLength, numIdentities); + Map randomIdentities = new HashMap(); + Set 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; } diff --git a/testutils/src/main/java/com/mparticle/testutils/RandomUtils.java b/testutils/src/main/java/com/mparticle/testutils/RandomUtils.java index 6714e2c61..b90a4ec59 100644 --- a/testutils/src/main/java/com/mparticle/testutils/RandomUtils.java +++ b/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 ASSIGNABLE_IDENTITY_TYPES = + assignableIdentityTypes(); + + private static List assignableIdentityTypes() { + List types = + new ArrayList(Arrays.asList(MParticle.IdentityType.values())); + types.remove(MParticle.IdentityType.Alias); + return Collections.unmodifiableList(types); + } + public Map 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 getRandomUserIdentities(Integer max) { - Map randomIdentities = new HashMap(); + 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 identityIndices = randomIntSet(0, identityTypeLength, numIdentities); + Map randomIdentities = new HashMap(); + Set 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; }