Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Don't remove listeners, which throw exceptions (fixes #209 and #395)

  • Loading branch information...
commit 0ce06206263ed060bd0ede049c1713286c2487ea 1 parent 1cec456
@stefanbirkner authored
View
47 src/main/java/org/junit/runner/notification/RunNotifier.java
@@ -1,5 +1,7 @@
package org.junit.runner.notification;
+import static java.util.Arrays.asList;
+
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
@@ -34,15 +36,30 @@ public void removeListener(RunListener listener) {
}
private abstract class SafeNotifier {
+ private final List<RunListener> fCurrentListeners;
+
+ SafeNotifier() {
+ this(fListeners);
+ }
+
+ SafeNotifier(List<RunListener> currentListeners) {
+ fCurrentListeners= currentListeners;
+ }
+
void run() {
synchronized (fListeners) {
- for (Iterator<RunListener> all= fListeners.iterator(); all.hasNext();)
+ List<RunListener> safeListeners= new ArrayList<RunListener>();
+ List<Failure> failures= new ArrayList<Failure>();
+ for (Iterator<RunListener> all= fCurrentListeners.iterator(); all
+ .hasNext();)
try {
- notifyListener(all.next());
+ RunListener listener= all.next();
+ notifyListener(listener);
+ safeListeners.add(listener);
} catch (Exception e) {
- all.remove(); // Remove the offending listener first to avoid an infinite loop
- fireTestFailure(new Failure(Description.TEST_MECHANISM, e));
+ failures.add(new Failure(Description.TEST_MECHANISM, e));
}
+ fireTestFailures(safeListeners, failures);
}
}
@@ -93,13 +110,21 @@ protected void notifyListener(RunListener each) throws Exception {
* Invoke to tell listeners that an atomic test failed.
* @param failure the description of the test that failed and the exception thrown
*/
- public void fireTestFailure(final Failure failure) {
- new SafeNotifier() {
- @Override
- protected void notifyListener(RunListener each) throws Exception {
- each.testFailure(failure);
- };
- }.run();
+ public void fireTestFailure(Failure failure) {
+ fireTestFailures(fListeners, asList(failure));
+ }
+
+ private void fireTestFailures(List<RunListener> listeners,
+ final List<Failure> failures) {
+ if (!failures.isEmpty())
+ new SafeNotifier(listeners) {
+ @Override
+ protected void notifyListener(RunListener listener)
+ throws Exception {
+ for (Failure each : failures)
+ listener.testFailure(each);
+ };
+ }.run();
}
/**
View
49 src/test/java/org/junit/runner/notification/RunNotifierTest.java
@@ -0,0 +1,49 @@
+package org.junit.runner.notification;
+
+import static org.junit.Assert.assertNotNull;
+import org.junit.Test;
+import org.junit.runner.Result;
+
+public class RunNotifierTest {
+
+ @Test
+ public void notifiesSecondListenerIfFirstThrowsException() {
+ FailureListener failureListener= new FailureListener();
+ RunNotifier notifier= new RunNotifier();
+ notifier.addListener(new CorruptListener());
+ notifier.addListener(failureListener);
+ notifier.fireTestFailure(new Failure(null, null));
+ assertNotNull("The FailureListener registered no failure.",
+ failureListener.failure);
+ }
+
+ @Test
+ public void hasNoProblemsWithFailingListeners() { // see issues 209 and 395
+ RunNotifier notifier= new RunNotifier();
+ notifier.addListener(new CorruptListener());
+ notifier.addListener(new FailureListener());
+ notifier.addListener(new CorruptListener());
+ notifier.fireTestRunFinished(new Result());
+ }
+
+ private static class CorruptListener extends RunListener {
+ @Override
+ public void testRunFinished(Result result) throws Exception {
+ throw new RuntimeException();
+ }
+
+ @Override
+ public void testFailure(Failure failure) throws Exception {
+ throw new RuntimeException();
+ }
+ }
+
+ private static class FailureListener extends RunListener {
+ private Failure failure;
+
+ @Override
+ public void testFailure(Failure failure) throws Exception {
+ this.failure= failure;
+ }
+ }
+}
View
7 src/test/java/org/junit/tests/listening/TestListenerTest.java
@@ -38,7 +38,8 @@ public void testRunStarted(Description description) throws Exception {
}
}
- @Test public void removeFailingListeners() {
+ @Test
+ public void reportsFailureOfListener() {
JUnitCore core= new JUnitCore();
core.addListener(new ExceptionListener());
@@ -48,10 +49,6 @@ public void testRunStarted(Description description) throws Exception {
assertEquals(1, result.getFailureCount());
Failure testFailure= result.getFailures().get(0);
assertEquals(Description.TEST_MECHANISM, testFailure.getDescription());
-
- count= 0;
- core.run(OneTest.class);
- assertEquals(0, count); // Doesn't change because listener was removed
}
@Test public void freshResultEachTime() {

1 comment on commit 0ce0620

@Stephan202

Didn't run the code, but it looks good to me. Thanks!

Please sign in to comment.
Something went wrong with that request. Please try again.