You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by dw...@apache.org on 2012/03/09 09:06:41 UTC
svn commit: r1298738 - in /lucene/dev/branches/branch_3x/lucene: ./
core/src/test/org/apache/lucene/util/junitcompat/
test-framework/src/java/org/apache/lucene/util/
Author: dweiss
Date: Fri Mar 9 08:06:41 2012
New Revision: 1298738
URL: http://svn.apache.org/viewvc?rev=1298738&view=rev
Log:
LUCENE-3857: exceptions from other threads in beforeclass/etc do not fail
the test (Dawid Weiss)
Conflicts:
lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
Added:
lucene/dev/branches/branch_3x/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestExceptionInBeforeClassHooks.java
lucene/dev/branches/branch_3x/lucene/test-framework/src/java/org/apache/lucene/util/UncaughtExceptionsRule.java
Modified:
lucene/dev/branches/branch_3x/lucene/CHANGES.txt
lucene/dev/branches/branch_3x/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
Modified: lucene/dev/branches/branch_3x/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/CHANGES.txt?rev=1298738&r1=1298737&r2=1298738&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/branch_3x/lucene/CHANGES.txt Fri Mar 9 08:06:41 2012
@@ -235,6 +235,9 @@ Documentation
Build
+* LUCENE-3857: exceptions from other threads in beforeclass/etc do not fail
+ the test (Dawid Weiss)
+
* LUCENE-3847: LuceneTestCase will now check for modifications of System
properties before and after each test (and suite). If changes are detected,
the test will fail. A rule can be used to reset system properties to
Added: lucene/dev/branches/branch_3x/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestExceptionInBeforeClassHooks.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestExceptionInBeforeClassHooks.java?rev=1298738&view=auto
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestExceptionInBeforeClassHooks.java (added)
+++ lucene/dev/branches/branch_3x/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestExceptionInBeforeClassHooks.java Fri Mar 9 08:06:41 2012
@@ -0,0 +1,96 @@
+package org.apache.lucene.util.junitcompat;
+
+import junit.framework.Assert;
+
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.JUnitCore;
+import org.junit.runner.Result;
+
+public class TestExceptionInBeforeClassHooks extends WithNestedTests {
+ public TestExceptionInBeforeClassHooks() {
+ super(true);
+ }
+
+ public static class Nested1 extends WithNestedTests.AbstractNestedTest {
+ @BeforeClass
+ public static void beforeClass() {
+ new Thread() {
+ public void run() {
+ throw new RuntimeException("foobar");
+ }
+ }.start();
+ }
+
+ public void test() {}
+ }
+
+ public static class Nested2 extends WithNestedTests.AbstractNestedTest {
+ public void test1() throws Exception {
+ Thread t = new Thread() {
+ public void run() {
+ throw new RuntimeException("foobar1");
+ }
+ };
+ t.start();
+ t.join();
+ }
+
+ public void test2() throws Exception {
+ Thread t = new Thread() {
+ public void run() {
+ throw new RuntimeException("foobar2");
+ }
+ };
+ t.start();
+ t.join();
+ }
+ }
+
+ public static class Nested3 extends WithNestedTests.AbstractNestedTest {
+ @Before
+ public void runBeforeTest() throws Exception {
+ Thread t = new Thread() {
+ public void run() {
+ throw new RuntimeException("foobar");
+ }
+ };
+ t.start();
+ t.join();
+ }
+
+ public void test1() throws Exception {
+ }
+ }
+
+ @Test
+ public void testExceptionInBeforeClassFailsTheTest() {
+ Result runClasses = JUnitCore.runClasses(Nested1.class);
+ Assert.assertEquals(1, runClasses.getFailureCount());
+ Assert.assertEquals(1, runClasses.getRunCount());
+ Assert.assertTrue(runClasses.getFailures().get(0).getTrace().contains("foobar"));
+ }
+
+ @Test
+ public void testExceptionWithinTestFailsTheTest() {
+ Result runClasses = JUnitCore.runClasses(Nested2.class);
+ Assert.assertEquals(2, runClasses.getFailureCount());
+ Assert.assertEquals(2, runClasses.getRunCount());
+
+ String m1 = runClasses.getFailures().get(0).getTrace();
+ String m2 = runClasses.getFailures().get(1).getTrace();
+ Assert.assertTrue(
+ (m1.contains("foobar1") && m2.contains("foobar2")) ||
+ (m1.contains("foobar2") && m2.contains("foobar1")));
+ }
+
+ @Test
+ public void testExceptionWithinBefore() {
+ Result runClasses = JUnitCore.runClasses(Nested3.class);
+ Assert.assertEquals(1, runClasses.getFailureCount());
+ Assert.assertEquals(1, runClasses.getRunCount());
+ Assert.assertTrue(runClasses.getFailures().get(0).getTrace().contains("foobar"));
+ }
+
+}
Modified: lucene/dev/branches/branch_3x/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java?rev=1298738&r1=1298737&r2=1298738&view=diff
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java (original)
+++ lucene/dev/branches/branch_3x/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java Fri Mar 9 08:06:41 2012
@@ -84,6 +84,7 @@ import org.junit.runner.Description;
import org.junit.runner.RunWith;
import org.junit.runner.Runner;
import org.junit.runner.notification.RunListener;
+import org.junit.runners.model.MultipleFailureException;
import org.junit.runners.model.Statement;
/**
@@ -176,7 +177,6 @@ public abstract class LuceneTestCase ext
private int savedBoolMaxClauseCount = BooleanQuery.getMaxClauseCount();
- private volatile Thread.UncaughtExceptionHandler savedUncaughtExceptionHandler = null;
/**
* @see SubclassSetupTeardownRule
*/
@@ -187,17 +187,6 @@ public abstract class LuceneTestCase ext
*/
private boolean teardownCalled;
- private static class UncaughtExceptionEntry {
- public final Thread thread;
- public final Throwable exception;
-
- public UncaughtExceptionEntry(Thread thread, Throwable exception) {
- this.thread = thread;
- this.exception = exception;
- }
- }
- private List<UncaughtExceptionEntry> uncaughtExceptions = Collections.synchronizedList(new ArrayList<UncaughtExceptionEntry>());
-
private static Locale locale;
private static Locale savedLocale;
private static TimeZone timeZone;
@@ -226,11 +215,37 @@ public abstract class LuceneTestCase ext
* Stores the currently class under test.
*/
private static final StoreClassNameRule classNameRule = new StoreClassNameRule();
-
+
+ /**
+ * Catch any uncaught exceptions on threads within the suite scope and fail the test/
+ * suite if they happen.
+ */
+ private static final UncaughtExceptionsRule uncaughtExceptionsRule = new UncaughtExceptionsRule();
+
+ /**
+ * This controls how suite-level rules are nested. It is important that _all_ rules declared
+ * in {@link LuceneTestCase} are executed in proper order if they depend on each
+ * other.
+ */
@ClassRule
public static TestRule classRules = RuleChain
.outerRule(new SystemPropertiesInvariantRule())
- .around(classNameRule);
+ .around(classNameRule)
+ .around(uncaughtExceptionsRule);
+
+ /**
+ * This controls how individual test rules are nested. It is important that _all_ rules declared
+ * in {@link LuceneTestCase} are executed in proper order if they depend on each
+ * other.
+ */
+ @Rule
+ public final TestRule ruleChain = RuleChain
+ .outerRule(new RememberThreadRule())
+ .around(new UncaughtExceptionsRule())
+ .around(new TestResultInterceptorRule())
+ .around(new SystemPropertiesInvariantRule())
+ .around(new InternalSetupTeardownRule())
+ .around(new SubclassSetupTeardownRule());
@BeforeClass
public static void beforeClassLuceneTestCaseJ4() {
@@ -335,6 +350,10 @@ public abstract class LuceneTestCase ext
if (problem != null) {
reportPartialFailureInfo();
}
+
+ if (uncaughtExceptionsRule.hasUncaughtExceptions()) {
+ testsFailed = true;
+ }
// if verbose or tests failed, report some information back
if (VERBOSE || testsFailed || problem != null) {
@@ -494,19 +513,6 @@ public abstract class LuceneTestCase ext
}
/**
- * This controls how rules are nested. It is important that _all_ rules declared
- * in {@link LuceneTestCase} are executed in proper order if they depend on each
- * other.
- */
- @Rule
- public final TestRule ruleChain = RuleChain
- .outerRule(new RememberThreadRule())
- .around(new TestResultInterceptorRule())
- .around(new SystemPropertiesInvariantRule())
- .around(new InternalSetupTeardownRule())
- .around(new SubclassSetupTeardownRule());
-
- /**
* Internal {@link LuceneTestCase} setup before/after each test.
*/
private class InternalSetupTeardownRule implements TestRule {
@@ -515,15 +521,26 @@ public abstract class LuceneTestCase ext
return new Statement() {
@Override
public void evaluate() throws Throwable {
- setUpInternal();
// We simulate the previous behavior of @Before in that
// if any statement below us fails, we just propagate the original
// exception and do not call tearDownInternal.
+ setUpInternal();
+ final ArrayList<Throwable> errors = new ArrayList<Throwable>();
+ try {
+ // But we will collect errors from statements below and wrap them
+ // into a multiple so that tearDownInternal is called.
+ base.evaluate();
+ } catch (Throwable t) {
+ errors.add(t);
+ }
+
+ try {
+ tearDownInternal();
+ } catch (Throwable t) {
+ errors.add(t);
+ }
- // TODO: [DW] should this really be this way? We could use
- // JUnit's MultipleFailureException and propagate both?
- base.evaluate();
- tearDownInternal();
+ MultipleFailureException.assertEmpty(errors);
}
};
}
@@ -536,36 +553,9 @@ public abstract class LuceneTestCase ext
seed = "random".equals(TEST_SEED) ? seedRand.nextLong() : ThreeLongs.fromString(TEST_SEED).l2;
random.setSeed(seed);
- savedUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler();
- Thread.setDefaultUncaughtExceptionHandler(new Thread.UncaughtExceptionHandler() {
- public void uncaughtException(Thread t, Throwable e) {
- // org.junit.internal.AssumptionViolatedException in older releases
- // org.junit.Assume.AssumptionViolatedException in recent ones
- if (e.getClass().getName().endsWith("AssumptionViolatedException")) {
- String where = "<unknown>";
- for (StackTraceElement elem : e.getStackTrace()) {
- if ( ! elem.getClassName().startsWith("org.junit")) {
- where = elem.toString();
- break;
- }
- }
- System.err.print("NOTE: Assume failed at " + where + " (ignored):");
- if (VERBOSE) {
- System.err.println();
- e.printStackTrace(System.err);
- } else {
- System.err.print(" ");
- System.err.println(e.getMessage());
- }
- } else {
- testsFailed = true;
- uncaughtExceptions.add(new UncaughtExceptionEntry(t, e));
- if (savedUncaughtExceptionHandler != null)
- savedUncaughtExceptionHandler.uncaughtException(t, e);
- }
- }
- });
-
+ Thread.currentThread().setName("LTC-main#seed=" +
+ new ThreeLongs(staticSeed, seed, LuceneTestCaseRunner.runnerSeed));
+
savedBoolMaxClauseCount = BooleanQuery.getMaxClauseCount();
}
@@ -674,15 +664,7 @@ public abstract class LuceneTestCase ext
// this won't throw any exceptions or fail the test
// if we change this, then change this logic
checkRogueThreadsAfter();
- // restore the default uncaught exception handler
- Thread.setDefaultUncaughtExceptionHandler(savedUncaughtExceptionHandler);
-
- try {
- checkUncaughtExceptionsAfter();
- } catch (Throwable t) {
- if (problem == null) problem = t;
- }
-
+
try {
// calling assertSaneFieldCaches here isn't as useful as having test
// classes call it directly from the scope where the index readers
@@ -709,7 +691,7 @@ public abstract class LuceneTestCase ext
throw new RuntimeException(problem);
}
}
-
+
/** check if the test still has threads running, we don't want them to
* fail in a subsequent test and pass the blame to the wrong test */
private void checkRogueThreadsAfter() {
@@ -718,26 +700,10 @@ public abstract class LuceneTestCase ext
if (!testsFailed && rogueThreads > 0) {
System.err.println("RESOURCE LEAK: test method: '" + getName()
+ "' left " + rogueThreads + " thread(s) running");
- // TODO: fail, but print seed for now
- if (uncaughtExceptions.isEmpty()) {
- reportAdditionalFailureInfo();
- }
}
}
}
- /** see if any other threads threw uncaught exceptions, and fail the test if so */
- private void checkUncaughtExceptionsAfter() {
- if (!uncaughtExceptions.isEmpty()) {
- System.err.println("The following exceptions were thrown by threads:");
- for (UncaughtExceptionEntry entry : uncaughtExceptions) {
- System.err.println("*** Thread: " + entry.thread.getName() + " ***");
- entry.exception.printStackTrace(System.err);
- }
- fail("Some threads threw uncaught exceptions!");
- }
- }
-
private final static int THREAD_STOP_GRACE_MSEC = 10;
// jvm-wide list of 'rogue threads' we found, so they only get reported once.
private final static IdentityHashMap<Thread,Boolean> rogueThreads = new IdentityHashMap<Thread,Boolean>();
Added: lucene/dev/branches/branch_3x/lucene/test-framework/src/java/org/apache/lucene/util/UncaughtExceptionsRule.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_3x/lucene/test-framework/src/java/org/apache/lucene/util/UncaughtExceptionsRule.java?rev=1298738&view=auto
==============================================================================
--- lucene/dev/branches/branch_3x/lucene/test-framework/src/java/org/apache/lucene/util/UncaughtExceptionsRule.java (added)
+++ lucene/dev/branches/branch_3x/lucene/test-framework/src/java/org/apache/lucene/util/UncaughtExceptionsRule.java Fri Mar 9 08:06:41 2012
@@ -0,0 +1,115 @@
+package org.apache.lucene.util;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.Thread.UncaughtExceptionHandler;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runners.model.MultipleFailureException;
+import org.junit.runners.model.Statement;
+
+/**
+ * Subscribes to
+ * {@link Thread#setDefaultUncaughtExceptionHandler(java.lang.Thread.UncaughtExceptionHandler)}
+ * and causes test/ suite failures if uncaught exceptions are detected.
+ */
+public class UncaughtExceptionsRule implements TestRule {
+ // This was originally volatile, but I don't think it needs to be. It's the same
+ // thread accessing it, always.
+ private UncaughtExceptionHandler savedUncaughtExceptionHandler;
+
+ public static class UncaughtExceptionEntry {
+ public final Thread thread;
+ public final Throwable exception;
+
+ public UncaughtExceptionEntry(Thread thread, Throwable exception) {
+ this.thread = thread;
+ this.exception = exception;
+ }
+ }
+
+ @SuppressWarnings("serial")
+ private static class UncaughtExceptionsInBackgroundThread extends RuntimeException {
+ public UncaughtExceptionsInBackgroundThread(UncaughtExceptionEntry e) {
+ super("Uncaught exception by thread: " + e.thread, e.exception);
+ }
+ }
+
+ // Lock on uncaughtExceptions to access.
+ private final List<UncaughtExceptionEntry> uncaughtExceptions = new ArrayList<UncaughtExceptionEntry>();
+
+ @Override
+ public Statement apply(final Statement s, final Description d) {
+ return new Statement() {
+ public void evaluate() throws Throwable {
+ final ArrayList<Throwable> errors = new ArrayList<Throwable>();
+ try {
+ setupHandler();
+ s.evaluate();
+ } catch (Throwable t) {
+ errors.add(t);
+ } finally {
+ restoreHandler();
+ }
+
+ synchronized (uncaughtExceptions) {
+ for (UncaughtExceptionEntry e : uncaughtExceptions) {
+ errors.add(new UncaughtExceptionsInBackgroundThread(e));
+ }
+ uncaughtExceptions.clear();
+ }
+
+ MultipleFailureException.assertEmpty(errors);
+ }
+ };
+ }
+
+ /**
+ * Just a check if anything's been caught.
+ */
+ public boolean hasUncaughtExceptions() {
+ synchronized (uncaughtExceptions) {
+ return !uncaughtExceptions.isEmpty();
+ }
+ }
+
+ private void restoreHandler() {
+ Thread.setDefaultUncaughtExceptionHandler(savedUncaughtExceptionHandler);
+ }
+
+ private void setupHandler() {
+ savedUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler();
+ Thread.setDefaultUncaughtExceptionHandler(new Thread.UncaughtExceptionHandler() {
+ public void uncaughtException(Thread t, Throwable e) {
+ // org.junit.internal.AssumptionViolatedException in older releases
+ // org.junit.Assume.AssumptionViolatedException in recent ones
+ if (e.getClass().getName().endsWith("AssumptionViolatedException")) {
+ String where = "<unknown>";
+ for (StackTraceElement elem : e.getStackTrace()) {
+ if (!elem.getClassName().startsWith("org.junit")) {
+ where = elem.toString();
+ break;
+ }
+ }
+ System.err.print("NOTE: Uncaught exception handler caught a failed assumption at "
+ + where + " (ignored):");
+ } else {
+ synchronized (uncaughtExceptions) {
+ uncaughtExceptions.add(new UncaughtExceptionEntry(t, e));
+ }
+
+ StringWriter sw = new StringWriter();
+ sw.write("\n===>\nUncaught exception by thread: " + t + "\n");
+ PrintWriter pw = new PrintWriter(sw);
+ e.printStackTrace(pw);
+ pw.flush();
+ sw.write("<===\n");
+ System.err.println(sw.toString());
+ }
+ }
+ });
+ }
+}