I've actually been trying to remove most of the Util and Helper classes
used by tests. My reasons are two-fold: 1) these classes tend to be very
non-OOP, 2) they tend to introduce a new API to the test which then
obscures what it's actually doing. When I look at a test, I don't want
anything hidden -- I want to SEE everything that the test is doing and
using, and I especially want to see the Geode user APIs that the test uses
(other Helper classes have hidden the user APIs). At the very least, please
try to steer away from using Util and Helper in a class name.

I would recommend a couple more changes. First off, decide what this object
is. For example, is it an Executor or ExecutorService? Should it be a JUnit
Rule instead of Util/Helper? Next simplify the API to match one of those.
See our ExecutorServiceRule which is in geode-junit. Is that similar to
what you're trying to create? Maybe we need just to
modify ExecutorServiceRule instead of introduce yet another Helper class. I
don't see why we need to add another class that is so similar to both
ExecutorServiceRule
and AsyncInvocation -- I vote to evolve those existing classes instead of
adding something new.

On Wed, 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();
>

Reply via email to