diff --git a/proctor-common/src/main/java/com/indeed/proctor/common/Proctor.java b/proctor-common/src/main/java/com/indeed/proctor/common/Proctor.java index 7a650709e..84b6585fe 100644 --- a/proctor-common/src/main/java/com/indeed/proctor/common/Proctor.java +++ b/proctor-common/src/main/java/com/indeed/proctor/common/Proctor.java @@ -342,24 +342,19 @@ public ProctorResult determineTestGroups( if (testChooser instanceof StandardTestChooser) { final TestType testType = testChooser.getTestDefinition().getTestType(); identifier = identifiers.getIdentifier(testType); - chooseResult = - ((StandardTestChooser) testChooser) - .choose( - identifier, - localContext, - testGroups, - forceGroupsOptions, - testTypesWithInvalidIdentifier); } else { - chooseResult = - ((RandomTestChooser) testChooser) - .choose( - localContext, - testGroups, - forceGroupsOptions, - identifiers.isRandomEnabled()); + identifier = null; } + chooseResult = + testChooser.choose( + identifier, + localContext, + testGroups, + forceGroupsOptions, + testTypesWithInvalidIdentifier, + identifiers.isRandomEnabled()); + if (chooseResult.getTestBucket() != null) { testGroups.put(testName, chooseResult.getTestBucket()); } diff --git a/proctor-common/src/main/java/com/indeed/proctor/common/RandomTestChooser.java b/proctor-common/src/main/java/com/indeed/proctor/common/RandomTestChooser.java index 0f5aef659..1d169668b 100644 --- a/proctor-common/src/main/java/com/indeed/proctor/common/RandomTestChooser.java +++ b/proctor-common/src/main/java/com/indeed/proctor/common/RandomTestChooser.java @@ -5,6 +5,7 @@ import com.indeed.proctor.common.model.ConsumableTestDefinition; import com.indeed.proctor.common.model.Range; import com.indeed.proctor.common.model.TestBucket; +import com.indeed.proctor.common.model.TestType; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -18,6 +19,7 @@ import java.util.List; import java.util.Map; import java.util.Random; +import java.util.Set; /** * Embodies the logic for a single purely random test, including applicability rule and @@ -58,10 +60,13 @@ private Map getDescriptorParameters() { } @Nonnull + @Override public TestChooser.Result choose( + @Nullable final String identifier, @Nonnull final Map localContext, @Nonnull final Map testGroups, @Nonnull final ForceGroupsOptions forceGroupsOptions, + @Nonnull final Set testTypesWithInvalidIdentifier, final boolean isRandomEnabled) { if (!isRandomEnabled) { // skipping here to make it use the fallback bucket. diff --git a/proctor-common/src/main/java/com/indeed/proctor/common/StandardTestChooser.java b/proctor-common/src/main/java/com/indeed/proctor/common/StandardTestChooser.java index 8e98d9b18..35d413f4e 100644 --- a/proctor-common/src/main/java/com/indeed/proctor/common/StandardTestChooser.java +++ b/proctor-common/src/main/java/com/indeed/proctor/common/StandardTestChooser.java @@ -47,12 +47,14 @@ public StandardTestChooser( } @Nonnull + @Override public TestChooser.Result choose( @Nullable final String identifier, @Nonnull final Map localContext, @Nonnull final Map testGroups, @Nonnull final ForceGroupsOptions forceGroupsOptions, - @Nonnull final Set testTypesWithInvalidIdentifier) { + @Nonnull final Set testTypesWithInvalidIdentifier, + final boolean isRandomEnabled) { final TestType testType = getTestDefinition().getTestType(); if (testTypesWithInvalidIdentifier.contains(testType)) { // skipping here to make it use the fallback bucket. diff --git a/proctor-common/src/main/java/com/indeed/proctor/common/TestChooser.java b/proctor-common/src/main/java/com/indeed/proctor/common/TestChooser.java index 0fa7cf5c8..cdc3eac15 100644 --- a/proctor-common/src/main/java/com/indeed/proctor/common/TestChooser.java +++ b/proctor-common/src/main/java/com/indeed/proctor/common/TestChooser.java @@ -6,6 +6,7 @@ import com.indeed.proctor.common.model.Payload; import com.indeed.proctor.common.model.Range; import com.indeed.proctor.common.model.TestBucket; +import com.indeed.proctor.common.model.TestType; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -17,6 +18,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.stream.Stream; interface TestChooser { @@ -45,6 +47,15 @@ TestChooser.Result chooseInternal( @Nonnull final Map localContext, @Nonnull Map testGroups); + @Nonnull + TestChooser.Result choose( + @Nullable final String identifier, + @Nonnull final Map localContext, + @Nonnull final Map testGroups, + @Nonnull final ForceGroupsOptions forceGroupsOptions, + @Nonnull final Set testTypesWithInvalidIdentifier, + final boolean isRandomEnabled); + @Nonnull default TestChooser.Result choose( @Nullable final IdentifierType identifier, diff --git a/proctor-common/src/test/java/com/indeed/proctor/common/TestProctor.java b/proctor-common/src/test/java/com/indeed/proctor/common/TestProctor.java index ba8e9d357..be8293baf 100644 --- a/proctor-common/src/test/java/com/indeed/proctor/common/TestProctor.java +++ b/proctor-common/src/test/java/com/indeed/proctor/common/TestProctor.java @@ -301,16 +301,20 @@ public void testDetermineTestGroupsForRandomTestWithRandomEnabled() { .thenReturn(result); when(testChooser.choose( + isNull(), eq(Collections.emptyMap()), anyMap(), eq(ForceGroupsOptions.empty()), + eq(Collections.emptySet()), eq(true))) .thenReturn(result); when(testChooser.choose( + isNull(), eq(Collections.emptyMap()), anyMap(), eq(ForceGroupsOptions.empty()), + eq(Collections.emptySet()), eq(false))) .thenReturn(TestChooser.Result.EMPTY); diff --git a/proctor-common/src/test/java/com/indeed/proctor/common/TestTestChooser.java b/proctor-common/src/test/java/com/indeed/proctor/common/TestTestChooser.java index 61f16ea65..f3dd36375 100644 --- a/proctor-common/src/test/java/com/indeed/proctor/common/TestTestChooser.java +++ b/proctor-common/src/test/java/com/indeed/proctor/common/TestTestChooser.java @@ -7,6 +7,7 @@ import com.indeed.proctor.common.model.Payload; import com.indeed.proctor.common.model.Range; import com.indeed.proctor.common.model.TestBucket; +import com.indeed.proctor.common.model.TestType; import org.junit.Test; import javax.annotation.Nonnull; @@ -18,6 +19,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; @@ -99,6 +101,18 @@ public String getTestName() { return "example_tst"; } + @Nonnull + @Override + public TestChooser.Result choose( + @Nullable final String identifier, + @Nonnull final Map localContext, + @Nonnull final Map testGroups, + @Nonnull final ForceGroupsOptions forceGroupsOptions, + @Nonnull final Set testTypesWithInvalidIdentifier, + final boolean isRandomEnabled) { + return TestChooser.super.choose(null, localContext, testGroups, forceGroupsOptions); + } + @Nonnull @Override public Result chooseInternal(