LUCENE-5567: When a suite fails with zombie threads failure marker and count is not propagated properly.

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1584569 13f79535-47bb-0310-9956-ffa450edef68

lucene/CHANGES.txt

@@ -233,6 +233,9 @@ Bug fixes
 
 Test Framework
 
+* LUCENE-5567: When a suite fails with zombie threads failure marker and count 
+  is not propagated properly. (Dawid Weiss)
+
 * LUCENE-5449: Rename _TestUtil and _TestHelper to remove the leading _.
 
 * LUCENE-5501: Added random out-of-order collection testing (when the collector

lucene/core/src/test/org/apache/lucene/util/TestMaxFailuresRule.java

@@ -17,8 +17,11 @@ package org.apache.lucene.util;
  * limitations under the License.
  */
 
+import java.util.concurrent.CountDownLatch;
+
 import org.apache.lucene.util.junitcompat.WithNestedTests;
 import org.junit.Assert;
+import org.junit.BeforeClass;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.Description;
@@ -27,8 +30,13 @@ import org.junit.runner.Result;
 import org.junit.runner.notification.Failure;
 import org.junit.runner.notification.RunListener;
 
-import com.carrotsearch.randomizedtesting.SysGlobals;
 import com.carrotsearch.randomizedtesting.annotations.Repeat;
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakAction;
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakLingering;
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope.Scope;
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakZombies;
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakZombies.Consequence;
 import com.carrotsearch.randomizedtesting.rules.SystemPropertiesInvariantRule;
 import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
 
@@ -66,48 +74,118 @@ public class TestMaxFailuresRule extends WithNestedTests {
 
   @Test
   public void testMaxFailures() {
-    TestRuleIgnoreAfterMaxFailures newRule = new TestRuleIgnoreAfterMaxFailures(2);
-    TestRuleIgnoreAfterMaxFailures prevRule = LuceneTestCase.replaceMaxFailureRule(newRule);
-    System.clearProperty(SysGlobals.SYSPROP_ITERATIONS());
-    try {
-      JUnitCore core = new JUnitCore();
-      final StringBuilder results = new StringBuilder();
-      core.addListener(new RunListener() {
-        char lastTest;
+    LuceneTestCase.replaceMaxFailureRule(new TestRuleIgnoreAfterMaxFailures(2));
+    JUnitCore core = new JUnitCore();
+    final StringBuilder results = new StringBuilder();
+    core.addListener(new RunListener() {
+      char lastTest;
 
-        @Override
-        public void testStarted(Description description) throws Exception {
-          lastTest = 'S'; // success.
-        }
+      @Override
+      public void testStarted(Description description) throws Exception {
+        lastTest = 'S'; // success.
+      }
 
-        @Override
-        public void testAssumptionFailure(Failure failure) {
-          lastTest = 'A'; // assumption failure.
-        }
+      @Override
+      public void testAssumptionFailure(Failure failure) {
+        lastTest = 'A'; // assumption failure.
+      }
 
-        @Override
-        public void testFailure(Failure failure) throws Exception {
-          lastTest = 'F'; // failure
-        }
+      @Override
+      public void testFailure(Failure failure) throws Exception {
+        lastTest = 'F'; // failure
+      }
 
-        @Override
-        public void testFinished(Description description) throws Exception {
-          results.append(lastTest);
-        }
-      });
+      @Override
+      public void testFinished(Description description) throws Exception {
+        results.append(lastTest);
+      }
+    });
 
-      Result result = core.run(Nested.class);
-      Assert.assertEquals(500, result.getRunCount());
-      Assert.assertEquals(0, result.getIgnoreCount());
-      Assert.assertEquals(2, result.getFailureCount());
+    Result result = core.run(Nested.class);
+    Assert.assertEquals(500, result.getRunCount());
+    Assert.assertEquals(0, result.getIgnoreCount());
+    Assert.assertEquals(2, result.getFailureCount());
 
-      // Make sure we had exactly two failures followed by assumption-failures
-      // resulting from ignored tests.
-      Assert.assertTrue(results.toString(), 
-          results.toString().matches("(S*F){2}A+"));
+    // Make sure we had exactly two failures followed by assumption-failures
+    // resulting from ignored tests.
+    Assert.assertTrue(results.toString(), 
+        results.toString().matches("(S*F){2}A+"));
+  }
 
-    } finally {
-      LuceneTestCase.replaceMaxFailureRule(prevRule);
+  @ThreadLeakZombies(Consequence.IGNORE_REMAINING_TESTS)
+  @ThreadLeakAction({ThreadLeakAction.Action.WARN})
+  @ThreadLeakScope(Scope.TEST)
+  @ThreadLeakLingering(linger = 500)
+  public static class Nested2 extends WithNestedTests.AbstractNestedTest {
+    public static final int TOTAL_ITERS = 10;
+    public static CountDownLatch die;
+    public static Thread zombie;
+    public static int testNum;
+    
+    @BeforeClass
+    public static void setup() {
+      assert zombie == null;
+      die = new CountDownLatch(1);
+      testNum = 0;
+    }
+
+    @Repeat(iterations = TOTAL_ITERS)
+    public void testLeaveZombie() {
+      if (++testNum == 2) {
+        zombie = new Thread() {
+          @Override
+          public void run() {
+            while (true) {
+              try {
+                die.await();
+                return;
+              } catch (Exception e) { /* ignore */ }
+            }
+          }
+        };
+        zombie.start();
+      }
     }
   }
+
+  @Test
+  public void testZombieThreadFailures() throws Exception {
+    LuceneTestCase.replaceMaxFailureRule(new TestRuleIgnoreAfterMaxFailures(1));
+    JUnitCore core = new JUnitCore();
+    final StringBuilder results = new StringBuilder();
+    core.addListener(new RunListener() {
+      char lastTest;
+
+      @Override
+      public void testStarted(Description description) throws Exception {
+        lastTest = 'S'; // success.
+      }
+
+      @Override
+      public void testAssumptionFailure(Failure failure) {
+        lastTest = 'A'; // assumption failure.
+      }
+
+      @Override
+      public void testFailure(Failure failure) throws Exception {
+        lastTest = 'F'; // failure
+        System.out.println(failure.getMessage());
+      }
+
+      @Override
+      public void testFinished(Description description) throws Exception {
+        results.append(lastTest);
+      }
+    });
+
+    Result result = core.run(Nested2.class);
+    if (Nested2.die != null) {
+      Nested2.die.countDown();
+      Nested2.zombie.join();
+    }
+    
+    super.prevSysOut.println(results.toString());
+    Assert.assertEquals(Nested2.TOTAL_ITERS, result.getRunCount());
+    Assert.assertEquals(results.toString(), "SFAAAAAAAA", results.toString());
+  }  
 }

lucene/core/src/test/org/apache/lucene/util/junitcompat/WithNestedTests.java

@@ -23,6 +23,7 @@ import java.io.UnsupportedEncodingException;
 import java.nio.charset.StandardCharsets;
 import java.util.List;
 
+import org.apache.lucene.util.FailureMarker;
 import org.apache.lucene.util.IOUtils;
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures;
@@ -37,6 +38,9 @@ import org.junit.Rule;
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 
+import com.carrotsearch.randomizedtesting.RandomizedRunner;
+import com.carrotsearch.randomizedtesting.RandomizedTest;
+import com.carrotsearch.randomizedtesting.SysGlobals;
 import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
 import com.carrotsearch.randomizedtesting.rules.TestRuleAdapter;
 
@@ -76,22 +80,36 @@ public abstract class WithNestedTests {
     private TestRuleIgnoreAfterMaxFailures prevRule;
 
     protected void before() throws Throwable {
-      String filter = System.getProperty("tests.filter");
-      if (filter != null && !filter.trim().isEmpty()) {
-        // We're running with a complex test filter. This will affect nested tests anyway
-        // so ignore them.
+      if (!isPropertyEmpty(SysGlobals.SYSPROP_TESTFILTER()) ||
+          !isPropertyEmpty(SysGlobals.SYSPROP_TESTCLASS())  ||
+          !isPropertyEmpty(SysGlobals.SYSPROP_TESTMETHOD()) ||
+          !isPropertyEmpty(SysGlobals.SYSPROP_ITERATIONS())) {
+        // We're running with a complex test filter that is properly handled by classes
+        // which are executed by RandomizedRunner. The "outer" classes testing LuceneTestCase
+        // itself are executed by the default JUnit runner and would be always executed.
+        // We thus always skip execution if any filtering is detected.
         Assume.assumeTrue(false);
       }
       
+      // Check zombie threads from previous suites. Don't run if zombies are around.
+      RandomizedTest.assumeFalse(RandomizedRunner.hasZombieThreads());
+
       TestRuleIgnoreAfterMaxFailures newRule = new TestRuleIgnoreAfterMaxFailures(Integer.MAX_VALUE);
       prevRule = LuceneTestCase.replaceMaxFailureRule(newRule);
+      RandomizedTest.assumeFalse(FailureMarker.hadFailures());
     }
 
     protected void afterAlways(List<Throwable> errors) throws Throwable {
       if (prevRule != null) {
         LuceneTestCase.replaceMaxFailureRule(prevRule);
       }
+      FailureMarker.resetFailures();
     }
+
+    private boolean isPropertyEmpty(String propertyName) {
+      String value = System.getProperty(propertyName);
+      return value == null || value.trim().isEmpty();
+    }    
   }); 
 
   /**
@@ -131,6 +149,7 @@ public abstract class WithNestedTests {
       }
     }
 
+    FailureMarker.resetFailures();
     System.setProperty(TestRuleIgnoreTestSuites.PROPERTY_RUN_NESTED, "true");
   }
 

lucene/ivy-versions.properties

@@ -8,7 +8,7 @@
 /cglib/cglib-nodep = 2.2
 /com.adobe.xmp/xmpcore = 5.1.2
 
-com.carrotsearch.randomizedtesting.version = 2.1.1
+com.carrotsearch.randomizedtesting.version = 2.1.3
 /com.carrotsearch.randomizedtesting/junit4-ant = ${com.carrotsearch.randomizedtesting.version}
 /com.carrotsearch.randomizedtesting/randomizedtesting-runner = ${com.carrotsearch.randomizedtesting.version}
 

lucene/licenses/junit4-ant-2.1.1.jar.sha1

@@ -1 +0,0 @@
-a8a7371e11a8b3a4a3eeea81ad3cedafe3e3550e

lucene/licenses/junit4-ant-2.1.3.jar.sha1

@@ -0,0 +1 @@
+8636804644d4ae3874f0efaa98978887e171cd55

lucene/licenses/randomizedtesting-runner-2.1.1.jar.sha1

@@ -1 +0,0 @@
-5908c4e714dab40ccc892993a21537c7c0d6210c

lucene/licenses/randomizedtesting-runner-2.1.3.jar.sha1

@@ -0,0 +1 @@
+d340caee99857ed0384681eea6219a4d937e7ee4

lucene/test-framework/src/java/org/apache/lucene/util/FailureMarker.java

@@ -0,0 +1,48 @@
+package org.apache.lucene.util;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.junit.runner.notification.Failure;
+import org.junit.runner.notification.RunListener;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * A {@link RunListener} that detects suite/ test failures. We need it because failures
+ * due to thread leaks happen outside of any rule contexts.
+ */
+public class FailureMarker extends RunListener {
+  static final AtomicInteger failures = new AtomicInteger();
+
+  @Override
+  public void testFailure(Failure failure) throws Exception {
+    failures.incrementAndGet();
+  }
+
+  public static boolean hadFailures() {
+    return failures.get() > 0;
+  }
+
+  static int getFailures() {
+    return failures.get();
+  }
+
+  public static void resetFailures() {
+    failures.set(0);
+  }
+}

lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java

@@ -126,7 +126,8 @@ import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAs
   JUnit4MethodProvider.class
 })
 @Listeners({
-  RunListenerPrintReproduceInfo.class
+  RunListenerPrintReproduceInfo.class,
+  FailureMarker.class
 })
 @SeedDecorators({MixWithSuiteName.class}) // See LUCENE-3995 for rationale.
 @ThreadLeakScope(Scope.SUITE)
@@ -355,8 +356,7 @@ public abstract class LuceneTestCase extends Assert {
   /**
    * Suite failure marker (any error in the test or suite scope).
    */
-  public final static TestRuleMarkFailure suiteFailureMarker = 
-      new TestRuleMarkFailure();
+  public static TestRuleMarkFailure suiteFailureMarker;
 
   /**
    * Ignore tests after hitting a designated number of initial failures. This
@@ -421,7 +421,7 @@ public abstract class LuceneTestCase extends Assert {
   public static TestRule classRules = RuleChain
     .outerRule(new TestRuleIgnoreTestSuites())
     .around(ignoreAfterMaxFailures)
-    .around(suiteFailureMarker)
+    .around(suiteFailureMarker = new TestRuleMarkFailure())
     .around(new TestRuleAssertionsRequired())
     .around(new StaticFieldsInvariantRule(STATIC_LEAK_THRESHOLD, true) {
       @Override

lucene/test-framework/src/java/org/apache/lucene/util/TestRuleIgnoreAfterMaxFailures.java

@@ -40,11 +40,6 @@ public final class TestRuleIgnoreAfterMaxFailures implements TestRule {
    * Maximum failures. Package scope for tests.
    */
   int maxFailures;
-
-  /**
-   * Current count of failures. Package scope for tests.
-   */
-  int failuresSoFar;
   
   /**
    * @param maxFailures
@@ -61,19 +56,13 @@ public final class TestRuleIgnoreAfterMaxFailures implements TestRule {
     return new Statement() {
       @Override
       public void evaluate() throws Throwable {
+        int failuresSoFar = FailureMarker.getFailures();
         if (failuresSoFar >= maxFailures) {
           RandomizedTest.assumeTrue("Ignored, failures limit reached (" + 
               failuresSoFar + " >= " + maxFailures + ").", false);
         }
 
-        try {
-          s.evaluate();
-        } catch (Throwable t) {
-          if (!TestRuleMarkFailure.isAssumption(t)) {
-            failuresSoFar++;
-          }
-          throw t;
-        }
+        s.evaluate();
       }
     };
   }

solr/licenses/junit4-ant-2.1.1.jar.sha1

@@ -1 +0,0 @@
-a8a7371e11a8b3a4a3eeea81ad3cedafe3e3550e

solr/licenses/junit4-ant-2.1.3.jar.sha1

@@ -0,0 +1 @@
+8636804644d4ae3874f0efaa98978887e171cd55

solr/licenses/randomizedtesting-runner-2.1.1.jar.sha1

@@ -1 +0,0 @@
-5908c4e714dab40ccc892993a21537c7c0d6210c

solr/licenses/randomizedtesting-runner-2.1.3.jar.sha1

@@ -0,0 +1 @@
+d340caee99857ed0384681eea6219a4d937e7ee4