Move wrapIfNotThreadSafe() to RunNotifier

Author: kcooney

src/main/java/org/junit/runner/notification/RunNotifier.java

@@ -31,7 +31,7 @@ public void addListener(RunListener listener) {
         if (listener == null) {
             throw new NullPointerException("Cannot add a null listener");
         }
-        fListeners.add(SynchronizedRunListener.wrapIfNotThreadSafe(listener));
+        fListeners.add(wrapIfNotThreadSafe(listener));
     }
 
     /**
@@ -41,9 +41,19 @@ public void removeListener(RunListener listener) {
         if (listener == null) {
             throw new NullPointerException("Cannot remove a null listener");
         }
-        fListeners.remove(SynchronizedRunListener.wrapIfNotThreadSafe(listener));
+        fListeners.remove(wrapIfNotThreadSafe(listener));
     }
 
+    /**
+     * Wraps the given listener with {@link SynchronizedRunListener} if
+     * it is not annotated with {@link RunListener.ThreadSafe}.
+     */
+    RunListener wrapIfNotThreadSafe(RunListener listener) {
+        return listener.getClass().isAnnotationPresent(RunListener.ThreadSafe.class) ?
+                listener : new SynchronizedRunListener(listener, this);
+    }
+
+
     private abstract class SafeNotifier {
         private final List<RunListener> fCurrentListeners;
 
@@ -52,7 +62,7 @@ private abstract class SafeNotifier {
         }
 
         SafeNotifier(List<RunListener> currentListeners) {
-            this.fCurrentListeners = currentListeners;
+            fCurrentListeners = currentListeners;
         }
 
         void run() {
@@ -201,6 +211,6 @@ public void addFirstListener(RunListener listener) {
         if (listener == null) {
             throw new NullPointerException("Cannot add a null listener");
         }
-        fListeners.add(0, SynchronizedRunListener.wrapIfNotThreadSafe(listener));
+        fListeners.add(0, wrapIfNotThreadSafe(listener));
     }
 }

src/main/java/org/junit/runner/notification/SynchronizedRunListener.java

@@ -7,7 +7,7 @@
  * Thread-safe decorator for {@link RunListener} implementations that synchronizes
  * calls to the delegate.
  *
- * <p>This class synchronizes all listener calls on a global monitor. This is done because
+ * <p>This class synchronizes all listener calls on a RunNotifier instance. This is done because
  * prior to JUnit 4.12, all listeners were called in a synchronized block in RunNotifier,
  * so no two listeners were ever called concurrently. If we instead made the methods here
  * sychronized, clients that added multiple listeners that called common code might see
@@ -21,67 +21,59 @@
  */
 @RunListener.ThreadSafe
 final class SynchronizedRunListener extends RunListener {
-    private static final Object sMonitor = new Object();
     private final RunListener fListener;
+    private final Object fMonitor;
 
-    /**
-     * Wraps the given listener with {@link SynchronizedRunListener} if
-     * it is not annotated with {@link RunListener.ThreadSafe}.
-     */
-    public static RunListener wrapIfNotThreadSafe(RunListener listener) {
-        return listener.getClass().isAnnotationPresent(RunListener.ThreadSafe.class) ?
-                listener : new SynchronizedRunListener(listener);
-    }
-
-    SynchronizedRunListener(RunListener listener) {
-        this.fListener = listener;
+    SynchronizedRunListener(RunListener listener, Object monitor) {
+        fListener = listener;
+        fMonitor = monitor;
     }
 
     @Override
     public void testRunStarted(Description description) throws Exception {
-        synchronized (sMonitor) {
+        synchronized (fMonitor) {
             fListener.testRunStarted(description);
         }
     }
 
     @Override
     public void testRunFinished(Result result) throws Exception {
-        synchronized (sMonitor) {
+        synchronized (fMonitor) {
             fListener.testRunFinished(result);
         }
     }
 
     @Override
     public void testStarted(Description description) throws Exception {
-        synchronized (sMonitor) {
+        synchronized (fMonitor) {
             fListener.testStarted(description);
         }
     }
 
     @Override
     public void testFinished(Description description) throws Exception {
-        synchronized (sMonitor) {
+        synchronized (fMonitor) {
             fListener.testFinished(description);
         }
     }
 
     @Override
     public void testFailure(Failure failure) throws Exception {
-        synchronized (sMonitor) {
+        synchronized (fMonitor) {
             fListener.testFailure(failure);
         }
     }
 
     @Override
     public void testAssumptionFailure(Failure failure) {
-        synchronized (sMonitor) {
+        synchronized (fMonitor) {
             fListener.testAssumptionFailure(failure);
         }
     }
 
     @Override
     public void testIgnored(Description description) throws Exception {
-        synchronized (sMonitor) {
+        synchronized (fMonitor) {
             fListener.testIgnored(description);
         }
     }
@@ -101,7 +93,7 @@ public boolean equals(Object other) {
         }
         SynchronizedRunListener that = (SynchronizedRunListener) other;
 
-        return this.fListener.equals(that.fListener);
+        return fListener.equals(that.fListener);
     }
 
     @Override

src/test/java/org/junit/runner/notification/RunNotifierTest.java

@@ -1,7 +1,9 @@
 package org.junit.runner.notification;
 
+import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThat;
 
 import java.util.concurrent.atomic.AtomicInteger;
@@ -91,6 +93,19 @@ public void addFirstAndRemoveWithThreadSafeListener() {
         assertThat(listener.fTestStarted.get(), is(1));
     }
 
+    @Test
+    public void wrapIfNotThreadSafeShouldNotWrapThreadSafeListeners() {
+        ThreadSafeListener listener = new ThreadSafeListener();;
+        assertSame(listener, new RunNotifier().wrapIfNotThreadSafe(listener));
+    }
+
+    @Test
+    public void wrapIfNotThreadSafeShouldWrapNonThreadSafeListeners() {
+        CountingListener listener = new CountingListener();
+        RunListener wrappedListener = new RunNotifier().wrapIfNotThreadSafe(listener);
+        assertThat(wrappedListener, instanceOf(SynchronizedRunListener.class));
+    }
+
     private static class FailureListener extends RunListener {
         private Failure failure;
 

src/test/java/org/junit/runner/notification/SynchronizedRunListenerTest.java

@@ -1,10 +1,8 @@
 package org.junit.runner.notification;
 
-import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
@@ -104,10 +102,6 @@ public boolean equals(Object obj) {
         }
     }
 
-    @RunListener.ThreadSafe
-    private static class ThreadSafeRunListener extends RunListener {
-    }
-
     @Test
     public void namedListenerCorrectlyImplementsEqualsAndHashCode() {
         NamedListener listener1 = new NamedListener("blue");
@@ -135,7 +129,7 @@ public void toStringDelegates() {
         NamedListener listener = new NamedListener("blue");
 
         assertEquals("NamedListener", listener.toString());
-        assertEquals("NamedListener (with synchronization wrapper)", new SynchronizedRunListener(listener).toString());
+        assertEquals("NamedListener (with synchronization wrapper)", wrap(listener).toString());
     }
 
     @Test
@@ -144,37 +138,20 @@ public void equalsDelegates() {
         NamedListener listener2 = new NamedListener("blue");
         NamedListener listener3 = new NamedListener("red");
 
-        assertEquals(new SynchronizedRunListener(listener1),
-                new SynchronizedRunListener(listener1));
-        assertEquals(new SynchronizedRunListener(listener1),
-                new SynchronizedRunListener(listener2));
-        assertNotEquals(new SynchronizedRunListener(listener1),
-                new SynchronizedRunListener(listener3));
-        assertNotEquals(new SynchronizedRunListener(listener1), listener1);
-        assertNotEquals(listener1, new SynchronizedRunListener(listener1));
+        assertEquals(wrap(listener1), wrap(listener1));
+        assertEquals(wrap(listener1), wrap(listener2));
+        assertNotEquals(wrap(listener1), wrap(listener3));
+        assertNotEquals(wrap(listener1), listener1);
+        assertNotEquals(listener1, wrap(listener1));
     }
 
     @Test
     public void hashCodeDelegates() {
         NamedListener listener = new NamedListener("blue");
-        assertEquals(listener.hashCode(), new SynchronizedRunListener(listener).hashCode());
-    }
-
-    @Test
-    public void wrapIfNotThreadSafeShouldNotWrapThreadSafeListeners() {
-        ThreadSafeRunListener listener = new ThreadSafeRunListener();;
-        assertSame(listener, SynchronizedRunListener.wrapIfNotThreadSafe(listener));
+        assertEquals(listener.hashCode(), wrap(listener).hashCode());
     }
 
-    /**
-     * Tests that {@link SynchronizedRunListener#wrapIfNotThreadSafe(RunListener)}
-     * wraps listeners that are not thread-safe. This does not check if the
-     * listener is synchronized; that is tested with the tests for {@link RunNotifier}.
-     */
-    @Test
-    public void wrapIfNotThreadSafeShouldWrapNonThreadSafeListeners() {
-        NamedListener listener = new NamedListener("name");
-        RunListener wrappedListener = SynchronizedRunListener.wrapIfNotThreadSafe(listener);
-        assertThat(wrappedListener, instanceOf(SynchronizedRunListener.class));
+    private SynchronizedRunListener wrap(RunListener listener) {
+        return new SynchronizedRunListener(listener, this);
     }
 }