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

Commit

Permalink
Merge pull request #157 from zacharygoodwin/PROC-1434
Browse files Browse the repository at this point in the history 
PROC-1434: Handle force groups for logging
  • Loading branch information
@zacharygoodwin
zacharygoodwin authored Feb 28, 2024
2 parents 3dc06dd + b731536 commit cf7a1e4
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 14 deletions.
10 changes: 8 additions & 2 deletions proctor-consumer/src/main/java/com/indeed/proctor/consumer/AbstractGroups.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,8 +436,14 @@ protected final void appendTestGroupsWithAllocations(
// no allocation might exist for this testbucket
final Allocation allocation =
proctorResult.getAllocations().get(testName);
if ((allocation != null)
&& !Strings.isNullOrEmpty(allocation.getId())) {
// null allocation equals forced group -> log in legacy format
// [test name + bucket value]
if (allocation == null) {
sb.append(testName)
.append(testBucket.getValue())
.append(separator);

} else if (!Strings.isNullOrEmpty(allocation.getId())) {
sb.append(allocation.getId())
.append(ALLOCATION_GROUP_SEPARATOR)
.append(testName)
Expand Down
10 changes: 6 additions & 4 deletions proctor-consumer/src/main/java/com/indeed/proctor/consumer/ProctorGroupsWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,13 @@ public final String writeGroupsAsString(

for (final TestGroupFormatter formatter : formatters) {
for (final String testName : filteredTestNames) {
// no allocation might exist for this testbucket
// no allocation might exist for this testbucket which represents a force group
final String allocId =
Optional.ofNullable(proctorResult.getAllocations().get(testName))
.map(Allocation::getId)
.orElse("");
proctorResult.getAllocations().get(testName) == null
? "force"
: Optional.ofNullable(proctorResult.getAllocations().get(testName))
.map(Allocation::getId)
.orElse("");
// allocation should never be null, guarding against NPE anyway
// id can be blank for historical data
final int lengthBefore = stringBuilder.length();
Expand Down
13 changes: 9 additions & 4 deletions proctor-consumer/src/main/java/com/indeed/proctor/consumer/logging/TestGroupFormatter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,19 @@ public interface TestGroupFormatter {

/**
* Appends test groups in the form with allocation ids as [allocation-id + ":" + test-name +
* bucket-value] for given test names. If allocation Id is empty, appends nothing
* bucket-value] for given test names. If force group append legacy format [test-name +
* bucket-value]. If allocation Id is empty, appends nothing
*/
TestGroupFormatter WITH_ALLOC_ID =
(sb, testName, allocationId, bucketValue) -> {
if (!StringUtils.isEmpty(allocationId)) {
sb.append(allocationId).append(DEFAULT_ALLOCATION_GROUP_SEPARATOR);
WITHOUT_ALLOC_ID.appendProctorTestGroup(
sb, testName, allocationId, bucketValue);
if (allocationId.equals("force")) {
WITHOUT_ALLOC_ID.appendProctorTestGroup(sb, testName, "", bucketValue);
} else {
sb.append(allocationId).append(DEFAULT_ALLOCATION_GROUP_SEPARATOR);
WITHOUT_ALLOC_ID.appendProctorTestGroup(
sb, testName, allocationId, bucketValue);
}
}
};

Expand Down
13 changes: 9 additions & 4 deletions proctor-consumer/src/test/java/com/indeed/proctor/consumer/ProctorGroupsWriterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public class ProctorGroupsWriterTest {
private static final String EMPTY_ALLOCID_DEFINITION_TEST_NAME = "c_empty_alloc_id";
private static final String GROUP1_TEST_NAME = "d_foo_tst";
private static final String SILENT_TEST_NAME = "e_silent_tst";
private static final String FORCED_TEST_NAME = "f_forced_tst";

// for this test, buckets can be reused between test definitions, sorting by testname
public static final TestBucket INACTIVE_BUCKET = new TestBucket("fooname", -1, "foodesc");
Expand All @@ -44,6 +45,7 @@ public class ProctorGroupsWriterTest {
.put(EMPTY_ALLOCID_DEFINITION_TEST_NAME, CONTROL_BUCKET)
.put(GROUP1_TEST_NAME, GROUP1_BUCKET)
.put(SILENT_TEST_NAME, GROUP1_BUCKET)
.put(FORCED_TEST_NAME, GROUP1_BUCKET)
.build();
private static final Map<String, Allocation> ALLOCATIONS =
new ImmutableSortedMap.Builder<String, Allocation>(Ordering.natural())
Expand All @@ -60,6 +62,7 @@ public class ProctorGroupsWriterTest {
.put(EMPTY_ALLOCID_DEFINITION_TEST_NAME, stubDefinition(GROUP1_BUCKET))
.put(GROUP1_TEST_NAME, stubDefinition(GROUP1_BUCKET))
.put(SILENT_TEST_NAME, stubDefinition(GROUP1_BUCKET, d -> d.setSilent(true)))
.put(FORCED_TEST_NAME, stubDefinition(GROUP1_BUCKET))
.build();

private static final ProctorResult PROCTOR_RESULT =
Expand All @@ -83,7 +86,7 @@ public void testWithEmptyResult() {
public void testDoubleFormattingWriter() {
// legacy Indeed behavior
final String expected =
"b_missing_definition0,c_empty_alloc_id0,d_foo_tst1,#A:b_missing_definition0,#A:d_foo_tst1";
"b_missing_definition0,c_empty_alloc_id0,d_foo_tst1,f_forced_tst1,#A:b_missing_definition0,#A:d_foo_tst1,f_forced_tst1";

final ProctorGroupsWriter defaultWriter =
new ProctorGroupsWriter.Builder(
Expand All @@ -97,8 +100,10 @@ public void testDoubleFormattingWriter() {
MISSING_DEFINITION_TEST_NAME + 0,
EMPTY_ALLOCID_DEFINITION_TEST_NAME + 0,
GROUP1_TEST_NAME + 1,
FORCED_TEST_NAME + 1,
"#A:" + MISSING_DEFINITION_TEST_NAME + 0,
"#A:" + GROUP1_TEST_NAME + 1)
"#A:" + GROUP1_TEST_NAME + 1,
FORCED_TEST_NAME + 1)
.with(","));
}

Expand All @@ -111,15 +116,15 @@ public void testCustomWriter() {
.setIncludeInactiveGroups(true)
.build();
assertThat(writerWithAllocIds.writeGroupsAsString(PROCTOR_RESULT))
.isEqualTo("#A:a_inactive_tst-1,#A:d_foo_tst1,#A:e_silent_tst1");
.isEqualTo("#A:a_inactive_tst-1,#A:d_foo_tst1,#A:e_silent_tst1,f_forced_tst1");

final ProctorGroupsWriter writerWithoutAllocIds =
new ProctorGroupsWriter.Builder(TestGroupFormatter.WITHOUT_ALLOC_ID)
.setIncludeSilentTests(true)
.setIncludeTestWithoutDefinition(false)
.build();
assertThat(writerWithoutAllocIds.writeGroupsAsString(PROCTOR_RESULT))
.isEqualTo("c_empty_alloc_id0,d_foo_tst1,e_silent_tst1");
.isEqualTo("c_empty_alloc_id0,d_foo_tst1,e_silent_tst1,f_forced_tst1");
}

@Test
Expand Down

0 comments on commit cf7a1e4

Please sign in to comment.