Skip to content
This repository has been archived by the owner on Jul 10, 2024. It is now read-only.

Commit

Permalink
PROC-1332: Refactor entry point to chooser
Browse files Browse the repository at this point in the history
  • Loading branch information
@zacharygoodwin
zacharygoodwin committed Oct 3, 2023
1 parent 45e585c commit 5a2bcdf
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 16 deletions.
25 changes: 10 additions & 15 deletions proctor-common/src/main/java/com/indeed/proctor/common/Proctor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
5 changes: 5 additions & 0 deletions proctor-common/src/main/java/com/indeed/proctor/common/RandomTestChooser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -58,10 +60,13 @@ private Map<String, String> getDescriptorParameters() {
}

@Nonnull
@Override
public TestChooser.Result choose(
@Nullable final String identifier,
@Nonnull final Map<String, ValueExpression> localContext,
@Nonnull final Map<String, TestBucket> testGroups,
@Nonnull final ForceGroupsOptions forceGroupsOptions,
@Nonnull final Set<TestType> testTypesWithInvalidIdentifier,
final boolean isRandomEnabled) {
if (!isRandomEnabled) {
// skipping here to make it use the fallback bucket.
Expand Down
4 changes: 3 additions & 1 deletion proctor-common/src/main/java/com/indeed/proctor/common/StandardTestChooser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,14 @@ public StandardTestChooser(
}

@Nonnull
@Override
public TestChooser.Result choose(
@Nullable final String identifier,
@Nonnull final Map<String, ValueExpression> localContext,
@Nonnull final Map<String, TestBucket> testGroups,
@Nonnull final ForceGroupsOptions forceGroupsOptions,
@Nonnull final Set<TestType> testTypesWithInvalidIdentifier) {
@Nonnull final Set<TestType> testTypesWithInvalidIdentifier,
final boolean isRandomEnabled) {
final TestType testType = getTestDefinition().getTestType();
if (testTypesWithInvalidIdentifier.contains(testType)) {
// skipping here to make it use the fallback bucket.
Expand Down
11 changes: 11 additions & 0 deletions proctor-common/src/main/java/com/indeed/proctor/common/TestChooser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<IdentifierType> {
Expand Down Expand Up @@ -45,6 +47,15 @@ TestChooser.Result chooseInternal(
@Nonnull final Map<String, ValueExpression> localContext,
@Nonnull Map<String, TestBucket> testGroups);

@Nonnull
TestChooser.Result choose(
@Nullable final String identifier,
@Nonnull final Map<String, ValueExpression> localContext,
@Nonnull final Map<String, TestBucket> testGroups,
@Nonnull final ForceGroupsOptions forceGroupsOptions,
@Nonnull final Set<TestType> testTypesWithInvalidIdentifier,
final boolean isRandomEnabled);

@Nonnull
default TestChooser.Result choose(
@Nullable final IdentifierType identifier,
Expand Down
4 changes: 4 additions & 0 deletions proctor-common/src/test/java/com/indeed/proctor/common/TestProctor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
14 changes: 14 additions & 0 deletions proctor-common/src/test/java/com/indeed/proctor/common/TestTestChooser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -99,6 +101,18 @@ public String getTestName() {
return "example_tst";
}

@Nonnull
@Override
public TestChooser.Result choose(
@Nullable final String identifier,
@Nonnull final Map<String, ValueExpression> localContext,
@Nonnull final Map<String, TestBucket> testGroups,
@Nonnull final ForceGroupsOptions forceGroupsOptions,
@Nonnull final Set<TestType> testTypesWithInvalidIdentifier,
final boolean isRandomEnabled) {
return TestChooser.super.choose(null, localContext, testGroups, forceGroupsOptions);
}

@Nonnull
@Override
public Result chooseInternal(
Expand Down

0 comments on commit 5a2bcdf

Please sign in to comment.