geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jacob Barrett <jbarr...@pivotal.io>
Subject Re: Proposed API for a ConcurrencyTestHelper
Date Wed, 18 Jul 2018 20:14:01 GMT
+1

Echo Dales suggestion for Duration over int for time.

-Jake


On Wed, Jul 18, 2018 at 12:58 PM Dan Smith <dsmith@pivotal.io> wrote:

> 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 <demery@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 <hbales@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
> > demery@pivotal.io
> >
> >
> >
> >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message