Hi Helena, Looks great! I think this will make writing multithreaded tests a lot less error prone. A couple of minor comments below:
I'm not sure we need runInSeries, someone could do that without using this test helper. Could we make the default timeout something more like 5 minutes? See this thread https://markmail.org/thread/yrcvdlbixqojl6oh -Dan On Wed, Jul 18, 2018 at 12:16 PM, Dale Emery <dem...@pivotal.io> wrote: > Hi Helena, > > I like it! > > A few suggestions: > > - For the durations (repeatForDuration() and setTimeout()), consider using > a java.time.Duration rather than an int to express the duration. The > Duration class lets most of the code not have to keep track of the time > units. The caller creates the Duration using whatever time unit they want. > When some other code needs to convert it to a specific time unit, there are > methods for that. The rest of the code doesn’t have to care about the time > units. And there are methods for adding, subtracting, and comparing > Durations and Instants, all handling the time units nicely. > > - Consider adding a version of runAndExpectValue that takes a Predicate > instead of a value. And maybe one taking a Hamcrest Matcher. > > - Consider specifying the type of the Callable, and preferably for the > value passed to runAndExpectValue()). And definitely for Predicates and > Matchers, if you add those methods. > > Cheers, > Dale > > > On Jul 18, 2018, at 11:53 AM, Helena Bales <hba...@pivotal.io> wrote: > > > > Hello All, > > > > Below I have included a proposed API for a concurrency test helper. The > > goal of this test helper is to make it easier for tests to use multiple > > threads. I believe providing a helper for this will reduce the number of > > tests that incorrectly use threads, and help make our DUnit tests more > > readable. > > > > I would like feedback on the API below. I have provided the method > > signatures that I plan on implementing, as well as an example usage. > Please > > let me know if there are any methods that seem unnecessary or if there > are > > any important use cases that I missed here. > > > > Thank you, > > Helena Bales > > > > public class ConcurrencyTestHelper { > > private List<Callable<T>> toRun; > > private Integer timeout; > > > > private final Integer DEFAULT_TIMEOUT = 30; > > > > /** > > * A default constructor that sets the timeout to a default of 30 > seconds > > */ > > public ConcurrencyTestHelper() { > > toRun = new ArrayList<Callable<T>>(); > > timeout = DEFAULT_TIMEOUT; > > } > > > > /** > > * Run the given Runnable and assert that an exception of the given > > type is thrown. The thread > > * will be cancelled when the class timeout is reached. > > * @param runnable a Runnable that throws an exception and responds > > to cancellation signal > > * @param exceptionType a Class of Exception that is expected to be > > thrown by the runnable > > */ > > public void runAndExpectException(Runnable runnable, Class > exceptionType); > > > > /** > > * Run the given Runnable and assert that no exceptions are thrown. > > The thread will be cancelled > > * when the class timeout is reached. > > * @param runnable a Runnable that does not throw an exception and > > responds to cancellation signal > > */ > > public void runAndExpectNoException(Runnable runnable); > > > > /** > > * Run the given Callable and assert that the result of that > > callable is the given expected value. > > * The thread will be cancelled when the class timeout is reached. > > * @param callable a Callable that returns an object, does not throw > > exceptions and responds to > > * cancellation signal > > * @param value an Object that is the expected return value from the > Callable. > > */ > > public void runAndExpectValue(Callable callable, Object value); > > > > /** > > * Run the given runnable in a loop until the rest of the threads > > complete. Fails if the runnable > > * throws an exception during any iteration. After the rest of the > > threads complete the loop will > > * not restart the runnable. The running iteration will be cancelled > > when the timeout set on the > > * class is reached. A cancellation will result in a test failure. > > * @param runnable a Runnable that does not throw an exception and > > responds to cancellation signal > > */ > > public void repeatUntilAllThreadsComplete(Runnable runnable); > > > > /** > > * Run the given runnable in a loop for some number of iterations. > > Fails if the runnable throws an > > * exception in any iteration. After the number of iterations has > > been met, the loop will not > > * restart the runnable. The running iteration of the Runnable will > > be cancelled when the > > * timeout set on the class is reached, regardless of if the number > > of desired iterations has > > * been reached. A cancellation will result in a test failure. > > * @param runnable a Runnable that does not throw an exception and > > responds to cancellation signal > > * @param iterations a maximum number of iterations to repeat for > > */ > > public void repeatForIterations(Runnable runnable, Integer iterations); > > > > /** > > * Run the given runnable in a loop for a certain duration. Fails if > > the runnable throws an > > * exception in any iteration. After the duration has been exceeded, > > the loop will not restart > > * the runnable, however the current iteration will not be cancelled > > until the timeout value for > > * the class has been reached. A cancellation will result in a test > failure. > > * @param runnable a Runnable that does not throw exceptions and > > responds to cancellation signal > > * @param duration a maximum amount of time to repeat for > > */ > > public void repeatForDuration(Runnable runnable, Integer duration); > > > > /** > > * Set the timeout for the threads. After the timeout is reached, > > the threads will be interrupted > > * and will throw a CancellationException > > */ > > public void setTimeout(Integer seconds) { > > timeout = seconds; > > } > > > > /** > > * Runs all callables in toRun in parallel and fail if threads' > > conditions were not met. Runs > > * until timeout is reached. > > * @throws InterruptedException if interrupted before timeout > > */ > > public void executeInParallel(); > > > > /** > > * Runs all callables in toRun in the order that they were added and > > fail if threads' conditions > > * were not met. Runs until timeout is reached. > > * @throws InterruptedException if interrupted before timeout > > */ > > public void executeInSeries(); > > } > > > > // example usage: > > new ConcurrentTestHelper() > > .runAndExpectException(runnable, UnsupportedOperationException. > class) > > .runAndExpectNoException(runnable) > > .runAndExpectValue(callable, result) > > .repeatUntilAllOtherRunnablesFinish(runnable) > > .repeatFor(runnable, time) //keep doing this for time amount of > time > > .repeat(runnable, iterations) //keep doing this for N times > > .setTimeout(seconds) //Throws exception if test takes too long > > .executeInParallel(); > > — > Dale Emery > dem...@pivotal.io > > > > >
