diff --git a/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestExceptionInBeforeClassHooks.java b/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestExceptionInBeforeClassHooks.java new file mode 100644 index 0000000..6d6234e --- /dev/null +++ b/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestExceptionInBeforeClassHooks.java @@ -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")); + } + +} diff --git a/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java b/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java index d9c09fe..fdb73b7 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java +++ b/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java @@ -104,6 +104,7 @@ import org.junit.internal.AssumptionViolatedException; import org.junit.rules.*; import org.junit.runner.*; import org.junit.runner.notification.RunListener; +import org.junit.runners.model.MultipleFailureException; import org.junit.runners.model.Statement; /** @@ -221,8 +222,6 @@ public abstract class LuceneTestCase extends Assert { private int savedBoolMaxClauseCount = BooleanQuery.getMaxClauseCount(); - private volatile Thread.UncaughtExceptionHandler savedUncaughtExceptionHandler = null; - /** * Some tests expect the directory to contain a single segment, and want to do tests on that segment's reader. * This is an utility method to help them. @@ -235,17 +234,6 @@ public abstract class LuceneTestCase extends Assert { return (SegmentReader) subReaders[0]; } - 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 uncaughtExceptions = Collections.synchronizedList(new ArrayList()); - // default codec private static Codec savedCodec; @@ -281,11 +269,37 @@ public abstract class LuceneTestCase extends Assert { * 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() { @@ -443,6 +457,10 @@ public abstract class LuceneTestCase extends Assert { if (problem != null) { reportPartialFailureInfo(); } + + if (uncaughtExceptionsRule.hasUncaughtExceptions()) { + testsFailed = true; + } // if verbose or tests failed, report some information back if (VERBOSE || testsFailed || problem != null) { @@ -608,19 +626,6 @@ public abstract class LuceneTestCase extends Assert { } /** - * 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 { @@ -629,15 +634,26 @@ public abstract class LuceneTestCase extends Assert { 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 errors = new ArrayList(); + 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); } }; } @@ -653,36 +669,6 @@ public abstract class LuceneTestCase extends Assert { Thread.currentThread().setName("LTC-main#seed=" + new ThreeLongs(staticSeed, seed, LuceneTestCaseRunner.runnerSeed)); - 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 = ""; - 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); - } - } - }); - savedBoolMaxClauseCount = BooleanQuery.getMaxClauseCount(); if (useNoMemoryExpensiveCodec) { @@ -781,15 +767,7 @@ public abstract class LuceneTestCase extends Assert { // 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 @@ -816,7 +794,7 @@ public abstract class LuceneTestCase extends Assert { 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() { @@ -825,26 +803,10 @@ public abstract class LuceneTestCase extends Assert { 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 rogueThreads = new IdentityHashMap(); diff --git a/lucene/test-framework/src/java/org/apache/lucene/util/UncaughtExceptionsRule.java b/lucene/test-framework/src/java/org/apache/lucene/util/UncaughtExceptionsRule.java new file mode 100644 index 0000000..82e5065 --- /dev/null +++ b/lucene/test-framework/src/java/org/apache/lucene/util/UncaughtExceptionsRule.java @@ -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 uncaughtExceptions = new ArrayList(); + + @Override + public Statement apply(final Statement s, final Description d) { + return new Statement() { + public void evaluate() throws Throwable { + final ArrayList errors = new ArrayList(); + 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 = ""; + 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()); + } + } + }); + } +}