Skip to content

Commit

Permalink
Changing the implementation of BackgroundThreadPosterTestDouble such …
Browse files Browse the repository at this point in the history 
…that it will not allow any posted Runnables to run before call to join().

This change is required in order to prevent some code to be executed while the tests that use BackgroundThreadPosterTestDouble are still being set up, thus leading to inconsistent results. Now, it's guaranteed that no side effects of Runnables posted BackgroundThreadPosterTestDouble will be visible until a call to join().
  • Loading branch information
@techyourchance
techyourchance committed Jun 12, 2019
1 parent a8a79ae commit d71e956
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 84 deletions.
2 changes: 1 addition & 1 deletion threadposter/src/main/java/com/techyourchance/threadposter/BackgroundThreadPoster.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public BackgroundThreadPoster() {
* Execute {@link Runnable} on a random background thread.
* @param runnable {@link Runnable} instance containing the code that should be executed
*/
public final void post(Runnable runnable) {
public void post(Runnable runnable) {
mThreadPoolExecutor.execute(runnable);
}

Expand Down
54 changes: 35 additions & 19 deletions ...in/java/com/techyourchance/threadposter/testdoubles/BackgroundThreadPosterTestDouble.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@

import java.util.LinkedList;
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.Semaphore;
import java.util.concurrent.SynchronousQueue;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;

/**
* Test double of {@link BackgroundThreadPoster} that can be used in tests in order to establish
Expand All @@ -19,12 +22,20 @@

private final Object MONITOR = new Object();

private final Queue<Thread> mThreads = new LinkedList<>();
private final Queue<Runnable> mRunnables = new ConcurrentLinkedQueue<>();

private int mNonCompletedRunnables = 0;

@Override
public void post(Runnable runnable) {
synchronized (MONITOR) {
mNonCompletedRunnables++;
}
mRunnables.add(runnable);
}

@Override
protected ThreadPoolExecutor newThreadPoolExecutor() {
// in order to support the strategy employed in join() method, we need to ensure that all
// threads are added to the queue, and are terminated the moment they are idle
return new ThreadPoolExecutor(
0,
Integer.MAX_VALUE,
Expand All @@ -33,12 +44,17 @@ protected ThreadPoolExecutor newThreadPoolExecutor() {
new SynchronousQueue<Runnable>(),
new ThreadFactory() {
@Override
public Thread newThread(Runnable r) {
synchronized (MONITOR) {
Thread newThread = new Thread(r);
mThreads.add(newThread);
return newThread;
}
public Thread newThread(final Runnable r) {
return new Thread(new Runnable() {
@Override
public void run() {
r.run();
synchronized (MONITOR) {
mNonCompletedRunnables--;
MONITOR.notifyAll();
}
}
});
}
}
);
Expand All @@ -51,17 +67,17 @@ public Thread newThread(Runnable r) {
* {@link Runnable}s sent for execution and any subsequent code.
*/
public void join() {
Queue<Thread> threadsCopy;
synchronized (MONITOR) {
threadsCopy = new LinkedList<>(mThreads);
}

Thread thread;
while ((thread = threadsCopy.poll()) != null) {
try {
thread.join();
} catch (InterruptedException e) {
e.printStackTrace();
Runnable runnable;
while (mNonCompletedRunnables > 0) {
while ((runnable = mRunnables.poll()) != null) {
super.post(runnable);
}
try {
MONITOR.wait();
} catch (InterruptedException e) {
throw new RuntimeException("interrupted");
}
}
}
}
Expand Down
103 changes: 41 additions & 62 deletions ...ava/com/techyourchance/threadposter/testdoubles/BackgroundThreadPosterTestDoubleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,10 @@

public class BackgroundThreadPosterTestDoubleTest {

private static final int TEST_TIMEOUT_MS = 1000;
private static final int TEST_DELAY_MS = TEST_TIMEOUT_MS / 10;
private static final int TEST_DELAY_MS = 10;

@ClassRule
public final static Timeout TIMEOUT = Timeout.millis(TEST_TIMEOUT_MS);

/**
* This class will be used in order to check side effects in tests
*/
private class Counter {

private AtomicInteger mCount = new AtomicInteger(0);

private void increment() {
mCount.incrementAndGet();
}

private int getCount() {
return mCount.get();
}
}
public final static Timeout TIMEOUT = Timeout.seconds(5);

private BackgroundThreadPosterTestDouble SUT;

Expand All @@ -50,11 +33,6 @@ public void executeThenJoin_singleRunnable_sideEffectsNotVisibleBeforeJoin() thr
Runnable runnable = new Runnable() {
@Override
public void run() {
try {
Thread.sleep(2 * TEST_DELAY_MS);
} catch (InterruptedException e) {
e.printStackTrace();
}
counter.increment();
}
};
Expand All @@ -72,18 +50,12 @@ public void executeThenJoin_singleRunnable_sideEffectsVisibleAfterJoin() throws
Runnable runnable = new Runnable() {
@Override
public void run() {
try {
Thread.sleep(2 * TEST_DELAY_MS);
} catch (InterruptedException e) {
e.printStackTrace();
}
counter.increment();
}
};
// Act
SUT.post(runnable);
// Assert
Thread.sleep(TEST_DELAY_MS);
SUT.join();
assertThat(counter.getCount(), is(1));
}
Expand All @@ -92,20 +64,21 @@ public void run() {
public void executeThenJoin_multipleRunnablesIndependent_sideEffectsNotVisibleBeforeJoin() throws Exception {
// Arrange
final Counter counter = new Counter();
Runnable runnable = new Runnable() {
Runnable runnable1 = new Runnable() {
@Override
public void run() {
counter.increment();
}
};
Runnable runnable2 = new Runnable() {
@Override
public void run() {
try {
Thread.sleep(2 * TEST_DELAY_MS);
} catch (InterruptedException e) {
e.printStackTrace();
}
counter.increment();
}
};
// Act
SUT.post(runnable);
SUT.post(runnable);
SUT.post(runnable1);
SUT.post(runnable2);
// Assert
Thread.sleep(TEST_DELAY_MS);
assertThat(counter.getCount(), is(0));
Expand All @@ -115,22 +88,22 @@ public void run() {
public void executeThenJoin_multipleRunnablesIndependent_sideEffectsVisibleAfterJoin() throws Exception {
// Arrange
final Counter counter = new Counter();
Runnable runnable = new Runnable() {
Runnable runnable1 = new Runnable() {
@Override
public void run() {
counter.increment();
}
};
Runnable runnable2 = new Runnable() {
@Override
public void run() {
try {
Thread.sleep(2 * TEST_DELAY_MS);
} catch (InterruptedException e) {
e.printStackTrace();
}
counter.increment();
}
};
// Act
SUT.post(runnable);
SUT.post(runnable);
SUT.post(runnable1);
SUT.post(runnable2);
// Assert
Thread.sleep(TEST_DELAY_MS);
SUT.join();
assertThat(counter.getCount(), is(2));
}
Expand All @@ -143,19 +116,14 @@ public void executeThenJoin_multipleRunnablesInterdependent_sideEffectsNotVisibl
Runnable runnable1 = new Runnable() {
@Override
public void run() {
try {
Thread.sleep(2 * TEST_DELAY_MS);
} catch (InterruptedException e) {
e.printStackTrace();
}
semaphore.release();
semaphore.acquireUninterruptibly();
counter.increment();
}
};
Runnable runnable2 = new Runnable() {
@Override
public void run() {
semaphore.acquireUninterruptibly();
semaphore.release();
counter.increment();
}
};
Expand All @@ -175,28 +143,39 @@ public void executeThenJoin_multipleRunnablesInterdependent_sideEffectsVisibleAf
Runnable runnable1 = new Runnable() {
@Override
public void run() {
try {
Thread.sleep(2 * TEST_DELAY_MS);
} catch (InterruptedException e) {
e.printStackTrace();
}
semaphore.release();
semaphore.acquireUninterruptibly();
counter.increment();
}
};
Runnable runnable2 = new Runnable() {
@Override
public void run() {
semaphore.acquireUninterruptibly();
semaphore.release();
counter.increment();
}
};
// Act
SUT.post(runnable1);
SUT.post(runnable2);
// Assert
Thread.sleep(TEST_DELAY_MS);
SUT.join();
assertThat(counter.getCount(), is(2));
}

/**
* This class will be used in order to check side effects in tests
*/
private class Counter {

private AtomicInteger mCount = new AtomicInteger(0);

private void increment() {
mCount.incrementAndGet();
}

private int getCount() {
return mCount.get();
}
}

}
7 changes: 5 additions & 2 deletions ...c/test/java/com/techyourchance/threadposter/testdoubles/UiThreadPosterTestDoubleTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package com.techyourchance.threadposter.testdoubles;

import com.techyourchance.threadposter.testdoubles.UiThreadPosterTestDouble;

import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.rules.Timeout;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.Is.is;
Expand All @@ -12,6 +12,9 @@ public class UiThreadPosterTestDoubleTest {

private static final int TEST_DELAY_MS = 10;

@ClassRule
public final static Timeout TIMEOUT = Timeout.seconds(5);

private UiThreadPosterTestDouble SUT;

@Before
Expand Down

0 comments on commit d71e956

Please sign in to comment.