HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Testing an Executor

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
executortestingstackoverflow

Problem

I have written the following Executor:

/**
 * This executor guarantees that there is never more than one element waiting to be executed. If an element is
 * submitted for execution and another element is still in queue, the newly submitted element replaces the previously
 * queued element in the queue
 *
 * Additionally this executor sleeps a constructor specified number of milliseconds before executing tasks to give queued
 * items a chance to be canceled
 *
 * This class is intended to be used when runnables are created in rapid succession and newer runnables supersede
 * ones that have been created previously. This is e.g. the case if a user interface reacts to user inputs, possibly
 * on every keystroke and the reaction causes slow tasks (e.g. server calls for validation).
 */
public class SingleQueuedElementExecutor implements Executor {
  private final Sleeper sleeper;
  private final ExecutorService delegate;
  private Future queuedSleeper;
  private Future queuedElement;

  public SingleQueuedElementExecutor(final ExecutorService delegate, final long sleepBeforeExecutionMilliseconds) {
    this.delegate = delegate;
    sleeper = new Sleeper(sleepBeforeExecutionMilliseconds);
  }

  public void execute(final Runnable runnable) {
    if (queuedElement != null) {
      queuedElement.cancel(false);
      queuedSleeper.cancel(true);
    }
    queuedSleeper = delegate.submit(sleeper);
    queuedElement = delegate.submit(runnable);
  }

  private static class Sleeper implements Runnable {
    private final long duration;

    private Sleeper(final long duration) {
      this.duration = duration;
    }

    public void run() {
      try {
        Thread.sleep(duration);
      } catch (InterruptedException e) {
        // Simply abort sleeping
      }
    }
  }
}


I believe that the JavaDoc describes accurately what I expect the class to do.

Now the interesting part which I ask a review about is the test for this class I came up with:

```
public class Sing

Solution

Currently your test is biased to help the unit under test to pass. The delegateExecutor is chosen to be a Executors.newSingleThreadExecutor(), which actually forces the SingleQueuedElementExecutor to finish with the Sleeper task before being able to start the actual task. Replace the delegateExecutor with Executors.newCachedThreadPool() and things get hairier.

Yet the class is helped in another way too : the timing times completion of all tasks, which incidently will include the Sleeper task. If you change the test to time until the start of SingleQueuedElementExecutorTest.InvocationCounter#run, the test will fail consistently.

Also you should note that one millisecond is 1000000 nanoseconds.

Since your unit under test uses Thread.sleep() it's hard to test without doing some stopwatch timing. That being said, it's not so hard to test the behavior you desire. What you basically want is that when the executorService is bombarded in short succession with new tasks that only the most recent one executes. That is actually not so hard to set up, and does not require awkard sleeping in the tests.
Another test could check that once a task has begun, a subsequently submitted task will also get executed (if not immediately followed by a slew of tasks itself). This test can simply make use of custom Runnables that force some timing using CountDownLatch instances.

Here's a sample :

public class SingleQueuedElementExecutorTest {

    private SingleQueuedElementExecutor executor;

    private ExecutorService delegateExecutor;
    private static final long SLEEP_BETWEEN_TASKS_MS = 10;
    private static final long NANO_TIME_ACCURACY_MS = 2;

    @Before
    public void setUp() throws Exception {
        delegateExecutor = Executors.newCachedThreadPool();
        executor = new SingleQueuedElementExecutor(delegateExecutor, SLEEP_BETWEEN_TASKS_MS);
    }

    @Test(timeout = 1000)
    public void testOnlyExecuteLastWhenTasksAreSubmittedInQuickSuccession() throws InterruptedException {
        final CountDownLatch lastExecuted = new CountDownLatch(1);
        final AtomicInteger executedCount = new AtomicInteger(0);
        for (int i = 0; i < 1000; i++) {
            executor.execute(new Runnable() {
                @Override
                public void run() {
                    executedCount.incrementAndGet();
                }
            });
        }
        executor.execute(new Runnable() {
            @Override
            public void run() {
                lastExecuted.countDown();
            }
        });
        lastExecuted.await();
        assertThat(executedCount.get()).isEqualTo(0); // all tasks preceding the last should have been cancelled
    }

    @Test(timeout = 1000)
    public void testSubmissionWhileExecutingPreviousDoesNotCancelPrevious() throws InterruptedException {
        final CountDownLatch startFirst = new CountDownLatch(1);
        final CountDownLatch secondSubmitted = new CountDownLatch(1);
        final CountDownLatch secondDone = new CountDownLatch(1);
        final AtomicBoolean firstExecuted = new AtomicBoolean(false);
        executor.execute(new Runnable() {
            @Override
            public void run() {
                startFirst.countDown();
                try {
                    secondSubmitted.await();
                    firstExecuted.set(true);
                } catch (InterruptedException e) {
                    Thread.currentThread().interrupt();
                }
            }
        });
        startFirst.await();
        executor.execute(new Runnable() {
            @Override
            public void run() {
                secondDone.countDown();
            }
        });
        secondDone.await();
        assertThat(firstExecuted.get()).isTrue();
    }

}

Code Snippets

public class SingleQueuedElementExecutorTest {

    private SingleQueuedElementExecutor executor;

    private ExecutorService delegateExecutor;
    private static final long SLEEP_BETWEEN_TASKS_MS = 10;
    private static final long NANO_TIME_ACCURACY_MS = 2;

    @Before
    public void setUp() throws Exception {
        delegateExecutor = Executors.newCachedThreadPool();
        executor = new SingleQueuedElementExecutor(delegateExecutor, SLEEP_BETWEEN_TASKS_MS);
    }

    @Test(timeout = 1000)
    public void testOnlyExecuteLastWhenTasksAreSubmittedInQuickSuccession() throws InterruptedException {
        final CountDownLatch lastExecuted = new CountDownLatch(1);
        final AtomicInteger executedCount = new AtomicInteger(0);
        for (int i = 0; i < 1000; i++) {
            executor.execute(new Runnable() {
                @Override
                public void run() {
                    executedCount.incrementAndGet();
                }
            });
        }
        executor.execute(new Runnable() {
            @Override
            public void run() {
                lastExecuted.countDown();
            }
        });
        lastExecuted.await();
        assertThat(executedCount.get()).isEqualTo(0); // all tasks preceding the last should have been cancelled
    }

    @Test(timeout = 1000)
    public void testSubmissionWhileExecutingPreviousDoesNotCancelPrevious() throws InterruptedException {
        final CountDownLatch startFirst = new CountDownLatch(1);
        final CountDownLatch secondSubmitted = new CountDownLatch(1);
        final CountDownLatch secondDone = new CountDownLatch(1);
        final AtomicBoolean firstExecuted = new AtomicBoolean(false);
        executor.execute(new Runnable() {
            @Override
            public void run() {
                startFirst.countDown();
                try {
                    secondSubmitted.await();
                    firstExecuted.set(true);
                } catch (InterruptedException e) {
                    Thread.currentThread().interrupt();
                }
            }
        });
        startFirst.await();
        executor.execute(new Runnable() {
            @Override
            public void run() {
                secondDone.countDown();
            }
        });
        secondDone.await();
        assertThat(firstExecuted.get()).isTrue();
    }

}

Context

StackExchange Code Review Q#32127, answer score: 2

Revisions (0)

No revisions yet.