Skip to content

Commit c099a99

Browse files
committed
Fix equals method for SynchronizedRunListener
1 parent 1d6914b commit c099a99

File tree

9 files changed

+77
-118
lines changed

9 files changed

+77
-118
lines changed

Diff for: .classpath

+1
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,6 @@
44
<classpathentry kind="src" path="src/test/java"/>
55
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/J2SE-1.5"/>
66
<classpathentry exported="true" kind="lib" path="lib/hamcrest-core-1.3.jar" sourcepath="lib/hamcrest-core-1.3-sources.jar"/>
7+
<classpathentry exported="true" kind="lib" path="lib/jcip-annotations-1.0.jar" sourcepath="jcip-annotations-1.0-sources.jar"/>
78
<classpathentry kind="output" path="bin"/>
89
</classpath>

Diff for: build.xml

+14-6
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
<property name="hamcrestlib" location="lib/hamcrest-core-1.3.jar" />
3232
<property name="hamcrestlibsources" location="lib/hamcrest-core-1.3-sources.jar" />
3333
<property name="hamcrestsrc" location="${dist}/temp.hamcrest.source" />
34+
<property name="jciplib" location="lib/jcip-annotations-1.0.jar" />
35+
<property name="jciplibsources" location="lib/jcip-annotations-1.0-sources.jar" />
3436

3537
<property name="maven.deploy.goal" value="org.apache.maven.plugins:maven-gpg-plugin:1.1:sign-and-deploy-file" />
3638

@@ -80,9 +82,10 @@
8082
</macrodef>
8183

8284
<target name="build" depends="versiontag">
83-
<junit_compilation srcdir="${src}" destdir="${bin}" classpath="${hamcrestlib}"/>
85+
<junit_compilation srcdir="${src}" destdir="${bin}" classpath="${hamcrestlib};${jciplib}"/>
8486
<unjar src="${hamcrestlib}" dest="${bin}" />
85-
<junit_compilation srcdir="${testsrc}" destdir="${testbin}" classpath="${hamcrestlib};${bin}"/>
87+
<unjar src="${jciplib}" dest="${bin}" />
88+
<junit_compilation srcdir="${testsrc}" destdir="${testbin}" classpath="${hamcrestlib};${jciplib};${bin}"/>
8689
</target>
8790

8891
<target name="jars" depends="build">
@@ -125,8 +128,9 @@
125128
</copy>
126129
</target>
127130

128-
<target name="unjar.hamcrest">
131+
<target name="unjar.deps">
129132
<unjar src="${hamcrestlibsources}" dest="${hamcrestsrc}" />
133+
<unjar src="${jciplibsources}" dest="${jcipsrc}" />
130134
</target>
131135

132136
<target name="release-notes">
@@ -138,20 +142,21 @@
138142
</exec>
139143
</target>
140144

141-
<target name="javadoc" depends="unjar.hamcrest">
145+
<target name="javadoc" depends="unjar.deps">
142146
<javadoc destdir="${javadocdir}"
143147
author="false"
144148
version="false"
145149
use="false"
146150
windowtitle="JUnit API"
147-
stylesheetfile="stylesheet.css"
151+
stylesheetfile="src/main/javadoc/stylesheet.css"
148152
>
149153
<excludepackage name="junit.*" />
150154
<excludepackage name="org.junit.internal.*" />
151155
<excludepackage name="org.junit.experimental.theories.internal.*" />
152156

153157
<sourcepath location="${src}" />
154158
<sourcepath location="${hamcrestsrc}" />
159+
<sourcepath location="${jcipsrc}" />
155160
<link href="http://java.sun.com/javase/6/docs/api/" />
156161
</javadoc>
157162
</target>
@@ -198,6 +203,7 @@
198203
<pathelement location="${bin}" />
199204
<pathelement location="${testbin}" />
200205
<pathelement location="${hamcrestlib}" />
206+
<pathelement location="${jciplib}" />
201207
</classpath>
202208
</java>
203209
</sequential>
@@ -207,12 +213,14 @@
207213
<run-local-tests />
208214
</target>
209215

210-
<target name="dist" depends="populate-dist">
216+
<target name="test-dist" depends="populate-dist">
211217
<run-dist-tests>
212218
<jvmarg value="-Dignore.this=ignored"/>
213219
</run-dist-tests>
214220
</target>
215221

222+
<target name="dist" depends="test-dist" />
223+
216224
<target name="profile" depends="populate-dist">
217225
<run-dist-tests>
218226
<jvmarg value="-agentlib:hprof=cpu=samples"/>

Diff for: lib/jcip-annotations-1.0-sources.jar

4.88 KB
Binary file not shown.

Diff for: lib/jcip-annotations-1.0.jar

2.2 KB
Binary file not shown.

Diff for: src/main/java/org/junit/experimental/runners/Enclosed.java

-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
* }
2424
* </pre>
2525
*
26-
* @see org.junit.tests.manipulation.SortableTest
2726
*/
2827
public class Enclosed extends Suite {
2928
/**

Diff for: src/main/java/org/junit/runner/notification/RunNotifier.java

+31-101
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
package org.junit.runner.notification;
22

3-
import net.jcip.annotations.ThreadSafe;
4-
import org.junit.internal.AssumptionViolatedException;
5-
import org.junit.runner.Description;
6-
import org.junit.runner.Result;
3+
import static java.util.Arrays.asList;
74

85
import java.util.ArrayList;
9-
import java.util.Arrays;
106
import java.util.Collection;
11-
import java.util.concurrent.locks.ReentrantLock;
7+
import java.util.List;
8+
import java.util.concurrent.CopyOnWriteArrayList;
9+
10+
import org.junit.internal.AssumptionViolatedException;
11+
import org.junit.runner.Description;
12+
import org.junit.runner.Result;
1213

1314
/**
1415
* If you write custom runners, you may need to notify JUnit of your progress running tests.
@@ -20,95 +21,37 @@
2021
* @since 4.0
2122
*/
2223
public class RunNotifier {
23-
private final ReentrantLock lock = new ReentrantLock();
24-
25-
private volatile RunListener[] listeners = new RunListener[0];
26-
private volatile boolean pleaseStop = false;
27-
28-
private static RunListener wrapSynchronizedIfNotThreadSafe(RunListener listener) {
29-
boolean isThreadSafe = listener.getClass().isAnnotationPresent(ThreadSafe.class);
30-
return isThreadSafe ? listener : new SynchronizedRunListener(listener);
31-
}
32-
33-
/**
34-
* Satisfies <tt>(o == null ? e == null : o.equals(e)</tt>
35-
* in {@link java.util.List#remove(Object)}.
36-
*
37-
* @param o listener to remove
38-
* @param e element in <code>listeners</code> which was previously added
39-
* @return {@code true} if <code>o</code> is equal with <code>e</code>
40-
*/
41-
private static boolean equalListeners(Object o, Object e) {
42-
if (o == null) {
43-
return e == null;
44-
} else {
45-
return e.getClass() == SynchronizedRunListener.class ? e.equals(o) : o.equals(e);
46-
}
47-
}
24+
private final List<RunListener> fListeners = new CopyOnWriteArrayList<RunListener>();
25+
private volatile boolean fPleaseStop = false;
4826

4927
/**
5028
* Internal use only
5129
*/
5230
public void addListener(RunListener listener) {
53-
if (listener != null) {
54-
listener = wrapSynchronizedIfNotThreadSafe(listener);
55-
final ReentrantLock lock = this.lock;
56-
lock.lock();
57-
try {
58-
// same behavior as List#add(Object)
59-
RunListener[] elements = this.listeners;
60-
int length = elements.length;
61-
RunListener[] listeners = new RunListener[1 + length];
62-
for (int i = 0; i < length; ++i) {
63-
listeners[i] = elements[i];
64-
}
65-
listeners[length] = listener;
66-
this.listeners = listeners;
67-
} finally {
68-
lock.unlock();
69-
}
70-
}
31+
if (listener == null) {
32+
throw new NullPointerException("Cannot add a null listener");
33+
}
34+
fListeners.add(SynchronizedRunListener.wrapIfNotThreadSafe(listener));
7135
}
7236

7337
/**
7438
* Internal use only
7539
*/
7640
public void removeListener(RunListener listener) {
77-
if (listener != null) {
78-
final ReentrantLock lock = this.lock;
79-
lock.lock();
80-
try {
81-
// same behavior as List#remove(Object)
82-
RunListener[] elements = this.listeners;
83-
int length = elements.length;
84-
if (length > 0) {
85-
RunListener[] listeners = new RunListener[length - 1];
86-
for (int i = 0, newLength = listeners.length; i < length; ++i) {
87-
if (equalListeners(listener, elements[i])) {
88-
for (int k = 1 + i; k != Integer.MAX_VALUE && k < length; ++k) {
89-
listeners[k - 1] = elements[k];
90-
}
91-
this.listeners = listeners;
92-
return;
93-
} else if (i < newLength) {
94-
listeners[i] = elements[i];
95-
}
96-
}
97-
}
98-
} finally {
99-
lock.unlock();
100-
}
101-
}
41+
if (listener == null) {
42+
throw new NullPointerException("Cannot remove a null listener");
43+
}
44+
fListeners.remove(SynchronizedRunListener.wrapIfNotThreadSafe(listener));
10245
}
10346

10447
private abstract class SafeNotifier {
105-
private final Collection<RunListener> currentListeners;
48+
private final List<RunListener> currentListeners;
10649

107-
SafeNotifier() {
108-
this(Arrays.asList(listeners));
50+
public SafeNotifier() {
51+
this(fListeners);
10952
}
11053

111-
SafeNotifier(Collection<RunListener> currentListeners) {
54+
public SafeNotifier(List<RunListener> currentListeners) {
11255
this.currentListeners = currentListeners;
11356
}
11457

@@ -134,7 +77,7 @@ void run() {
13477
* Do not invoke.
13578
*/
13679
public void fireTestRunStarted(final Description description) {
137-
new SafeNotifier() {
80+
new SafeNotifier(fListeners) {
13881
@Override
13982
protected void notifyListener(RunListener each) throws Exception {
14083
each.testRunStarted(description);
@@ -161,7 +104,7 @@ protected void notifyListener(RunListener each) throws Exception {
161104
* @throws StoppedByUserException thrown if a user has requested that the test run stop
162105
*/
163106
public void fireTestStarted(final Description description) throws StoppedByUserException {
164-
if (pleaseStop) {
107+
if (fPleaseStop) {
165108
throw new StoppedByUserException();
166109
}
167110
new SafeNotifier() {
@@ -178,10 +121,10 @@ protected void notifyListener(RunListener each) throws Exception {
178121
* @param failure the description of the test that failed and the exception thrown
179122
*/
180123
public void fireTestFailure(Failure failure) {
181-
fireTestFailures(Arrays.asList(listeners), Arrays.asList(failure));
124+
fireTestFailures(fListeners, asList(failure));
182125
}
183126

184-
private void fireTestFailures(Collection<RunListener> listeners, final Collection<Failure> failures) {
127+
private void fireTestFailures(List<RunListener> listeners, final Collection<Failure> failures) {
185128
if (!failures.isEmpty()) {
186129
new SafeNotifier(listeners) {
187130
@Override
@@ -247,29 +190,16 @@ protected void notifyListener(RunListener each) throws Exception {
247190
* to be shared amongst the many runners involved.
248191
*/
249192
public void pleaseStop() {
250-
pleaseStop = true;
193+
fPleaseStop = true;
251194
}
252195

253196
/**
254197
* Internal use only. The Result's listener must be first.
255198
*/
256199
public void addFirstListener(RunListener listener) {
257-
if (listener != null) {
258-
listener = wrapSynchronizedIfNotThreadSafe(listener);
259-
final ReentrantLock lock = this.lock;
260-
lock.lock();
261-
try {
262-
// same behavior as List#add(0, Object)
263-
RunListener[] elements = this.listeners;
264-
RunListener[] listeners = new RunListener[1 + elements.length];
265-
listeners[0] = listener;
266-
for (int i = 0, length = elements.length; i < length; ++i) {
267-
listeners[1 + i] = elements[i];
268-
}
269-
this.listeners = listeners;
270-
} finally {
271-
lock.unlock();
272-
}
273-
}
200+
if (listener == null) {
201+
throw new NullPointerException("Cannot remove a null listener");
202+
}
203+
fListeners.add(0, SynchronizedRunListener.wrapIfNotThreadSafe(listener));
274204
}
275-
}
205+
}

Diff for: src/main/java/org/junit/runner/notification/SynchronizedRunListener.java

+16-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.junit.runner.notification;
22

3+
import net.jcip.annotations.ThreadSafe;
34
import org.junit.runner.Description;
45
import org.junit.runner.Result;
56

@@ -14,6 +15,11 @@
1415
final class SynchronizedRunListener extends RunListener {
1516
private final RunListener listener;
1617

18+
public static RunListener wrapIfNotThreadSafe(RunListener listener) {
19+
boolean isThreadSafe = listener.getClass().isAnnotationPresent(ThreadSafe.class);
20+
return isThreadSafe ? listener : new SynchronizedRunListener(listener);
21+
}
22+
1723
SynchronizedRunListener(RunListener listener) {
1824
this.listener = listener;
1925
}
@@ -59,8 +65,16 @@ public int hashCode() {
5965
}
6066

6167
@Override
62-
public boolean equals(Object o) {
63-
return o.equals(listener);
68+
public boolean equals(Object other) {
69+
if (this == other) {
70+
return true;
71+
}
72+
if (!(other instanceof SynchronizedRunListener)) {
73+
return false;
74+
}
75+
SynchronizedRunListener that= (SynchronizedRunListener) other;
76+
77+
return this.listener.equals(that.listener);
6478
}
6579

6680
@Override

Diff for: src/test/java/org/junit/runner/notification/AddRemoveListenerTest.java

+12-7
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,29 @@
11
package org.junit.runner.notification;
22

3-
import net.jcip.annotations.ThreadSafe;
4-
import org.junit.Test;
5-
import org.junit.runner.Description;
3+
import static org.hamcrest.core.Is.is;
4+
import static org.junit.Assert.assertThat;
65

76
import java.util.concurrent.atomic.AtomicInteger;
87

9-
import static org.hamcrest.core.Is.is;
10-
import static org.junit.Assert.assertThat;
8+
import net.jcip.annotations.ThreadSafe;
9+
import org.junit.Test;
10+
import org.junit.runner.Description;
11+
import org.junit.runner.RunWith;
12+
import org.junit.runners.JUnit4;
1113

1214
/**
1315
* @author Tibor Digana (tibor17)
1416
* @version 4.12
1517
* @since 4.12
1618
*/
19+
@RunWith(JUnit4.class)
1720
public class AddRemoveListenerTest {
1821

1922
public static class NormalListener extends RunListener {
2023
final AtomicInteger testStarted = new AtomicInteger(0);
2124

22-
public void testStarted(Description description) throws Exception {
25+
@Override
26+
public void testStarted(Description description) throws Exception {
2327
testStarted.incrementAndGet();
2428
}
2529
}
@@ -28,7 +32,8 @@ public void testStarted(Description description) throws Exception {
2832
public static class ThreadSafeListener extends RunListener {
2933
final AtomicInteger testStarted = new AtomicInteger(0);
3034

31-
public void testStarted(Description description) throws Exception {
35+
@Override
36+
public void testStarted(Description description) throws Exception {
3237
testStarted.incrementAndGet();
3338
}
3439
}

Diff for: src/test/java/org/junit/tests/AllTests.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import org.junit.internal.MethodSorterTest;
66
import org.junit.internal.matchers.StacktracePrintingMatcherTest;
77
import org.junit.runner.RunWith;
8+
import org.junit.runner.notification.AddRemoveListenerTest;
89
import org.junit.runner.notification.RunNotifierTest;
910
import org.junit.runner.notification.ConcurrentRunNotifierTest;
1011
import org.junit.runners.Suite;
@@ -170,7 +171,8 @@
170171
StacktracePrintingMatcherTest.class,
171172
StopwatchTest.class,
172173
RunNotifierTest.class,
173-
ConcurrentRunNotifierTest.class
174+
ConcurrentRunNotifierTest.class,
175+
AddRemoveListenerTest.class
174176
})
175177
public class AllTests {
176178
public static Test suite() {

0 commit comments

Comments
 (0)