From 639898ec07da2a9022557c61a1d6f3b962f81357 Mon Sep 17 00:00:00 2001 From: Jon Chambers Date: Fri, 10 Jul 2020 17:51:11 -0400 Subject: [PATCH] Expand Experiment to deal with async suppliers and Optionals. --- .../textsecuregcm/experiment/Experiment.java | 41 ++++-- .../experiment/ExperimentTest.java | 123 +++++++++++------- 2 files changed, 108 insertions(+), 56 deletions(-) diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/experiment/Experiment.java b/service/src/main/java/org/whispersystems/textsecuregcm/experiment/Experiment.java index 0418a0add..a808c61d5 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/experiment/Experiment.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/experiment/Experiment.java @@ -7,7 +7,11 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.Objects; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; import java.util.function.Supplier; import static com.codahale.metrics.MetricRegistry.name; @@ -77,24 +81,37 @@ public class Experiment { } } + public void compareSupplierResultAsync(final T expected, final Supplier experimentSupplier, final Executor executor) { + try { + compareFutureResult(expected, CompletableFuture.supplyAsync(experimentSupplier, executor)); + } catch (final Exception e) { + recordError(e); + } + } + private void recordError(final Throwable cause) { log.warn("Experiment {} threw an exception.", name, cause); errorCounter.increment(); } - private void recordResult(final T expected, final T actual) { - final Counter counter; - - if (Objects.equals(expected, actual)) { - counter = matchCounter; - } else if (expected == null) { - counter = controlNullMismatchCounter; - } else if (actual == null) { - counter = experimentNullMismatchCounter; + @VisibleForTesting + void recordResult(final T expected, final T actual) { + if (expected instanceof Optional && actual instanceof Optional) { + recordResult(((Optional)expected).orElse(null), ((Optional)actual).orElse(null)); } else { - counter = bothPresentMismatchCounter; - } + final Counter counter; - counter.increment(); + if (Objects.equals(expected, actual)) { + counter = matchCounter; + } else if (expected == null) { + counter = controlNullMismatchCounter; + } else if (actual == null) { + counter = experimentNullMismatchCounter; + } else { + counter = bothPresentMismatchCounter; + } + + counter.increment(); + } } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/experiment/ExperimentTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/experiment/ExperimentTest.java index 21f35e4a9..c939f5ca2 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/experiment/ExperimentTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/experiment/ExperimentTest.java @@ -1,21 +1,32 @@ package org.whispersystems.textsecuregcm.experiment; import io.micrometer.core.instrument.Counter; +import junitparams.JUnitParamsRunner; +import junitparams.Parameters; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import java.util.List; +import java.util.Optional; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.TimeUnit; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +@RunWith(JUnitParamsRunner.class) public class ExperimentTest { private Counter matchCounter; private Counter errorCounter; - private Counter bothPresentMismatchCounter; - private Counter controlNullMismatchCounter; - private Counter experimentNullMismatchCounter; private Experiment experiment; @@ -23,37 +34,16 @@ public class ExperimentTest { public void setUp() { matchCounter = mock(Counter.class); errorCounter = mock(Counter.class); - bothPresentMismatchCounter = mock(Counter.class); - controlNullMismatchCounter = mock(Counter.class); - experimentNullMismatchCounter = mock(Counter.class); - experiment = new Experiment("test", matchCounter, errorCounter, bothPresentMismatchCounter, controlNullMismatchCounter, experimentNullMismatchCounter); + experiment = new Experiment("test", matchCounter, errorCounter, mock(Counter.class), mock(Counter.class), mock(Counter.class)); } @Test - public void compareFutureResultMatch() { + public void compareFutureResult() { experiment.compareFutureResult(12, CompletableFuture.completedFuture(12)); verify(matchCounter).increment(); } - @Test - public void compareFutureResultMismatchBothPresent() { - experiment.compareFutureResult(12, CompletableFuture.completedFuture(77)); - verify(bothPresentMismatchCounter).increment(); - } - - @Test - public void compareFutureResultMismatchControlNull() { - experiment.compareFutureResult(null, CompletableFuture.completedFuture(77)); - verify(controlNullMismatchCounter).increment(); - } - - @Test - public void compareFutureResultMismatchExperimentNull() { - experiment.compareFutureResult(12, CompletableFuture.completedFuture(null)); - verify(experimentNullMismatchCounter).increment(); - } - @Test public void compareFutureResultError() { experiment.compareFutureResult(12, CompletableFuture.failedFuture(new RuntimeException("OH NO"))); @@ -66,27 +56,72 @@ public class ExperimentTest { verify(matchCounter).increment(); } - @Test - public void compareSupplierResultMismatchBothPresent() { - experiment.compareSupplierResult(12, () -> 77); - verify(bothPresentMismatchCounter).increment(); - } - - @Test - public void compareSupplierResultMismatchControlNull() { - experiment.compareSupplierResult(null, () -> 77); - verify(controlNullMismatchCounter).increment(); - } - - @Test - public void compareSupplierResultMismatchExperimentNull() { - experiment.compareSupplierResult(12, () -> null); - verify(experimentNullMismatchCounter).increment(); - } - @Test public void compareSupplierResultError() { experiment.compareSupplierResult(12, () -> { throw new RuntimeException("OH NO"); }); verify(errorCounter).increment(); } + + @Test + public void compareSupplierResultAsyncMatch() throws InterruptedException { + final ExecutorService experimentExecutor = Executors.newSingleThreadExecutor(); + + experiment.compareSupplierResultAsync(12, () -> 12, experimentExecutor); + experimentExecutor.shutdown(); + experimentExecutor.awaitTermination(1, TimeUnit.SECONDS); + + verify(matchCounter).increment(); + } + + @Test + public void compareSupplierResultAsyncError() throws InterruptedException { + final ExecutorService experimentExecutor = Executors.newSingleThreadExecutor(); + + experiment.compareSupplierResultAsync(12, () -> { throw new RuntimeException("OH NO"); }, experimentExecutor); + experimentExecutor.shutdown(); + experimentExecutor.awaitTermination(1, TimeUnit.SECONDS); + + verify(errorCounter).increment(); + } + + @Test + public void compareSupplierResultAsyncRejection() { + final ExecutorService executorService = mock(ExecutorService.class); + doThrow(new RejectedExecutionException()).when(executorService).execute(any(Runnable.class)); + + experiment.compareSupplierResultAsync(12, () -> 12, executorService); + verify(errorCounter).increment(); + } + + @Test + @Parameters(method = "argumentsForTestRecordResult") + public void testRecordResult(final Object expected, final Object actual, final Experiment experiment, final Counter expectedCounter) { + reset(expectedCounter); + + experiment.recordResult(expected, actual); + verify(expectedCounter).increment(); + } + + @SuppressWarnings("unused") + private Object argumentsForTestRecordResult() { + // Hack: parameters are set before the @Before method gets called + final Counter matchCounter = mock(Counter.class); + final Counter errorCounter = mock(Counter.class); + final Counter bothPresentMismatchCounter = mock(Counter.class); + final Counter controlNullMismatchCounter = mock(Counter.class); + final Counter experimentNullMismatchCounter = mock(Counter.class); + + final Experiment experiment = new Experiment("test", matchCounter, errorCounter, bothPresentMismatchCounter, controlNullMismatchCounter, experimentNullMismatchCounter); + + return new Object[] { + new Object[] { 12, 12, experiment, matchCounter }, + new Object[] { null, 12, experiment, controlNullMismatchCounter }, + new Object[] { 12, null, experiment, experimentNullMismatchCounter }, + new Object[] { 12, 17, experiment, bothPresentMismatchCounter }, + new Object[] { Optional.of(12), Optional.of(12), experiment, matchCounter }, + new Object[] { Optional.empty(), Optional.of(12), experiment, controlNullMismatchCounter }, + new Object[] { Optional.of(12), Optional.empty(), experiment, experimentNullMismatchCounter }, + new Object[] { Optional.of(12), Optional.of(17), experiment, bothPresentMismatchCounter } + }; + } }