Skip to content

Commit a04aae1

Browse files
kcooneykcooney
committed
Add tests in AddRemoveTestListenerTest to RunNotifierTest
1 parent 5112c1a commit a04aae1

File tree

3 files changed

+89
-23
lines changed

3 files changed

+89
-23
lines changed

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ private static Class<? extends Annotation> loadThreadSafeAnnotation() {
134134
*
135135
* @return the annotation or {@code null} if it isn't on the classpath
136136
*/
137-
static Class<? extends Annotation> getThreadSafeAnnotationClass() {
138-
return ThreadSafeAnnotationHolder.annotation;
139-
}
137+
static Class<? extends Annotation> getThreadSafeAnnotationClass() {
138+
return ThreadSafeAnnotationHolder.annotation;
139+
}
140140
}

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

+14-18
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,8 @@
66
import java.util.concurrent.atomic.AtomicInteger;
77

88
import net.jcip.annotations.ThreadSafe;
9-
import org.junit.Ignore;
109
import org.junit.Test;
1110
import org.junit.runner.Description;
12-
import org.junit.runner.RunWith;
13-
import org.junit.runners.JUnit4;
1411

1512
/**
1613
* TODO: Move these tests to {@link RunNotifierTest}?
@@ -19,7 +16,6 @@
1916
* @version 4.12
2017
* @since 4.12
2118
*/
22-
@RunWith(JUnit4.class)
2319
public class AddRemoveListenerTest {
2420
private final RunNotifier notifier = new RunNotifier();
2521

@@ -42,13 +38,13 @@ public void testStarted(Description description) throws Exception {
4238
}
4339
}
4440

45-
/**
46-
* Disabled for now because it uses a listener with an equals()
47-
* method that violates the contract of equals().
48-
* This does pass, but it's unclear whether we need it; the
49-
* functionality appears to be tested in {@link SynchronizedRunListenerTest}.
50-
*/
51-
@Test @Ignore
41+
/**
42+
* TODO: Replace with a better test. This test uses a listener with an
43+
* equals() method that violates the contract of equals(). This does pass,
44+
* but it's unclear whether we need it; the functionality appears to be
45+
* tested in {@link SynchronizedRunListenerTest} and {@link RunNotifierTest}.
46+
*/
47+
@Test
5248
public void keepContractOnEqualsNegative() {
5349
final NormalListener listener = new NormalListener();
5450
NormalListener wrappedListener = new NormalListener() {
@@ -66,13 +62,13 @@ public boolean equals(Object o) {
6662
assertThat(wrappedListener.testStarted.get(), is(2));
6763
}
6864

69-
/**
70-
* Disabled for now because it uses a listener with an equals()
71-
* method that violates the contract of equals().
72-
* This does pass, but it's unclear whether we need it; the
73-
* functionality appears to be tested in {@link SynchronizedRunListenerTest}.
74-
*/
75-
@Test @Ignore
65+
/**
66+
* TODO: Replace with a better test. This test uses a listener with an
67+
* equals() method that violates the contract of equals(). This does pass,
68+
* but it's unclear whether we need it; the functionality appears to be
69+
* tested in {@link SynchronizedRunListenerTest} and {@link RunNotifierTest}.
70+
*/
71+
@Test
7672
public void keepContractOnEquals() {
7773
final NormalListener listener = new NormalListener();
7874
NormalListener wrappedListener = new NormalListener() {

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

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

3+
import static org.hamcrest.core.Is.is;
4+
import static org.junit.Assert.assertThat;
5+
6+
import java.util.concurrent.atomic.AtomicInteger;
7+
38
import junit.framework.TestCase;
9+
import net.jcip.annotations.ThreadSafe;
10+
import org.junit.runner.Description;
411
import org.junit.runner.Result;
512

613
/**
@@ -9,10 +16,16 @@
916
* other tests (including causing failed tests to appear to be passing).
1017
*/
1118
public class RunNotifierTest extends TestCase {
19+
private RunNotifier notifier;
20+
21+
@Override
22+
protected void setUp() throws Exception {
23+
super.setUp();
24+
notifier = new RunNotifier();
25+
}
1226

1327
public void testNotifiesSecondListenerIfFirstThrowsException() {
1428
FailureListener failureListener = new FailureListener();
15-
RunNotifier notifier = new RunNotifier();
1629
notifier.addListener(new CorruptListener());
1730
notifier.addListener(failureListener);
1831
notifier.fireTestFailure(new Failure(null, null));
@@ -21,7 +34,6 @@ public void testNotifiesSecondListenerIfFirstThrowsException() {
2134
}
2235

2336
public void testHasNoProblemsWithFailingListeners() { // see issues 209 and 395
24-
RunNotifier notifier = new RunNotifier();
2537
notifier.addListener(new CorruptListener());
2638
notifier.addListener(new FailureListener());
2739
notifier.addListener(new CorruptListener());
@@ -39,6 +51,50 @@ public void testFailure(Failure failure) throws Exception {
3951
throw new RuntimeException();
4052
}
4153
}
54+
55+
public void testAddAndRemoveWithNonThreadSafeListener() {
56+
CountingListener listener = new CountingListener();
57+
assertThat(listener.testStarted.get(), is(0));
58+
notifier.addListener(listener);
59+
notifier.fireTestStarted(null);
60+
assertThat(listener.testStarted.get(), is(1));
61+
notifier.removeListener(listener);
62+
notifier.fireTestStarted(null);
63+
assertThat(listener.testStarted.get(), is(1));
64+
}
65+
66+
public void testAddFirstAndRemoveWithNonThreadSafeListener() {
67+
CountingListener listener = new CountingListener();
68+
assertThat(listener.testStarted.get(), is(0));
69+
notifier.addFirstListener(listener);
70+
notifier.fireTestStarted(null);
71+
assertThat(listener.testStarted.get(), is(1));
72+
notifier.removeListener(listener);
73+
notifier.fireTestStarted(null);
74+
assertThat(listener.testStarted.get(), is(1));
75+
}
76+
77+
public void testAddAndRemoveWithThreadSafeListener() {
78+
ThreadSafeListener listener = new ThreadSafeListener();
79+
assertThat(listener.testStarted.get(), is(0));
80+
notifier.addListener(listener);
81+
notifier.fireTestStarted(null);
82+
assertThat(listener.testStarted.get(), is(1));
83+
notifier.removeListener(listener);
84+
notifier.fireTestStarted(null);
85+
assertThat(listener.testStarted.get(), is(1));
86+
}
87+
88+
public void testAddFirstAndRemoveWithThreadSafeListener() {
89+
ThreadSafeListener listener = new ThreadSafeListener();
90+
assertThat(listener.testStarted.get(), is(0));
91+
notifier.addFirstListener(listener);
92+
notifier.fireTestStarted(null);
93+
assertThat(listener.testStarted.get(), is(1));
94+
notifier.removeListener(listener);
95+
notifier.fireTestStarted(null);
96+
assertThat(listener.testStarted.get(), is(1));
97+
}
4298

4399
private static class FailureListener extends RunListener {
44100
private Failure failure;
@@ -48,4 +104,18 @@ public void testFailure(Failure failure) throws Exception {
48104
this.failure = failure;
49105
}
50106
}
107+
108+
private static class CountingListener extends RunListener {
109+
final AtomicInteger testStarted = new AtomicInteger(0);
110+
111+
@Override
112+
public void testStarted(Description description) throws Exception {
113+
testStarted.incrementAndGet();
114+
}
115+
}
116+
117+
@ThreadSafe
118+
private static class ThreadSafeListener extends CountingListener {
119+
}
120+
51121
}

0 commit comments

Comments
 (0)