LUCENE-4997: Internal test framework's tests are sensitive to previous test failures and tests.failfast.

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

lucene/CHANGES.txt

@@ -78,6 +78,9 @@ Changes in backwards compatibility policy
 
 Bug Fixes
 
+* LUCENE-4997: Internal test framework's tests are sensitive to previous 
+  test failures and tests.failfast. (Dawid Weiss, Shai Erera)
+
 * LUCENE-4935: CustomScoreQuery wrongly applied its query boost twice 
   (boost^2).  (Robert Muir)
 

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

@@ -66,13 +66,10 @@ public class TestMaxFailuresRule extends WithNestedTests {
 
   @Test
   public void testMaxFailures() {
-    int maxFailures = LuceneTestCase.ignoreAfterMaxFailures.maxFailures;
-    int failuresSoFar = LuceneTestCase.ignoreAfterMaxFailures.failuresSoFar;
+    TestRuleIgnoreAfterMaxFailures newRule = new TestRuleIgnoreAfterMaxFailures(2);
+    TestRuleIgnoreAfterMaxFailures prevRule = LuceneTestCase.replaceMaxFailureRule(newRule);
     System.clearProperty(SysGlobals.SYSPROP_ITERATIONS());
     try {
-      LuceneTestCase.ignoreAfterMaxFailures.maxFailures = 2;
-      LuceneTestCase.ignoreAfterMaxFailures.failuresSoFar = 0;
-
       JUnitCore core = new JUnitCore();
       final StringBuilder results = new StringBuilder();
       core.addListener(new RunListener() {
@@ -110,8 +107,7 @@ public class TestMaxFailuresRule extends WithNestedTests {
           results.toString().matches("(S*F){2}A+"));
 
     } finally {
-      LuceneTestCase.ignoreAfterMaxFailures.maxFailures = maxFailures;
-      LuceneTestCase.ignoreAfterMaxFailures.failuresSoFar = failuresSoFar;
+      LuceneTestCase.replaceMaxFailureRule(prevRule);
     }
   }
 }

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

@@ -23,11 +23,13 @@ import java.io.UnsupportedEncodingException;
 import java.util.List;
 
 import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util.TestRuleIgnoreAfterMaxFailures;
 import org.apache.lucene.util.TestRuleIgnoreTestSuites;
 import org.apache.lucene.util.TestRuleMarkFailure;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
+import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
@@ -66,6 +68,22 @@ public abstract class WithNestedTests {
   private ByteArrayOutputStream sysout;
   private ByteArrayOutputStream syserr;
 
+  @ClassRule
+  public static final TestRule classRules = RuleChain.outerRule(new TestRuleAdapter() {
+    private TestRuleIgnoreAfterMaxFailures prevRule;
+
+    protected void before() throws Throwable {
+      TestRuleIgnoreAfterMaxFailures newRule = new TestRuleIgnoreAfterMaxFailures(Integer.MAX_VALUE);
+      prevRule = LuceneTestCase.replaceMaxFailureRule(newRule);
+    }
+
+    protected void afterAlways(List<Throwable> errors) throws Throwable {
+      if (prevRule != null) {
+        LuceneTestCase.replaceMaxFailureRule(prevRule);
+      }
+    }
+  }); 
+
   /**
    * Restore properties after test.
    */
@@ -86,7 +104,7 @@ public abstract class WithNestedTests {
       })
       .around(marker);
   }
-      
+
   @Before
   public final void before() {
     if (suppressOutputStreams) {

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

@@ -23,6 +23,7 @@ import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
 import java.util.*;
 import java.util.concurrent.*;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.logging.Logger;
 
 import org.apache.lucene.analysis.Analyzer;
@@ -148,10 +149,10 @@ public abstract class LuceneTestCase extends Assert {
   public static final String SYSPROP_BADAPPLES = "tests.badapples";
 
   /** @see #ignoreAfterMaxFailures*/
-  private static final String SYSPROP_MAXFAILURES = "tests.maxfailures";
+  public static final String SYSPROP_MAXFAILURES = "tests.maxfailures";
 
   /** @see #ignoreAfterMaxFailures*/
-  private static final String SYSPROP_FAILFAST = "tests.failfast";
+  public static final String SYSPROP_FAILFAST = "tests.failfast";
 
   /**
    * Annotation for tests that should only be run during nightly builds.
@@ -356,9 +357,17 @@ public abstract class LuceneTestCase extends Assert {
       new TestRuleMarkFailure();
 
   /**
-   * Ignore tests after hitting a designated number of initial failures.
+   * Ignore tests after hitting a designated number of initial failures. This
+   * is truly a "static" global singleton since it needs to span the lifetime of all
+   * test classes running inside this JVM (it cannot be part of a class rule).
+   * 
+   * <p>This poses some problems for the test framework's tests because these sometimes
+   * trigger intentional failures which add up to the global count. This field contains
+   * a (possibly) changing reference to {@link TestRuleIgnoreAfterMaxFailures} and we
+   * dispatch to its current value from the {@link #classRules} chain using {@link TestRuleDelegate}.  
    */
-  final static TestRuleIgnoreAfterMaxFailures ignoreAfterMaxFailures; 
+  private static final AtomicReference<TestRuleIgnoreAfterMaxFailures> ignoreAfterMaxFailuresDelegate;
+  private static final TestRule ignoreAfterMaxFailures;
   static {
     int maxFailures = systemPropertyAsInt(SYSPROP_MAXFAILURES, Integer.MAX_VALUE);
     boolean failFast = systemPropertyAsBoolean(SYSPROP_FAILFAST, false);
@@ -373,7 +382,19 @@ public abstract class LuceneTestCase extends Assert {
       }
     }
 
-    ignoreAfterMaxFailures = new TestRuleIgnoreAfterMaxFailures(maxFailures);
+    ignoreAfterMaxFailuresDelegate = 
+        new AtomicReference<TestRuleIgnoreAfterMaxFailures>(
+            new TestRuleIgnoreAfterMaxFailures(maxFailures));
+    ignoreAfterMaxFailures = TestRuleDelegate.of(ignoreAfterMaxFailuresDelegate);
+  }
+
+  /**
+   * Temporarily substitute the global {@link TestRuleIgnoreAfterMaxFailures}. See
+   * {@link #ignoreAfterMaxFailuresDelegate} for some explanation why this method 
+   * is needed.
+   */
+  public static TestRuleIgnoreAfterMaxFailures replaceMaxFailureRule(TestRuleIgnoreAfterMaxFailures newValue) {
+    return ignoreAfterMaxFailuresDelegate.getAndSet(newValue);
   }
 
   /**

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

@@ -0,0 +1,45 @@
+package org.apache.lucene.util;
+
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runners.model.Statement;
+
+/*
+ * 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 TestRule} that delegates to another {@link TestRule} via a delegate
+ * contained in a an {@link AtomicReference}.
+ */
+final class TestRuleDelegate<T extends TestRule> implements TestRule {
+  private AtomicReference<T> delegate;
+
+  private TestRuleDelegate(AtomicReference<T> delegate) {
+    this.delegate = delegate;
+  }
+
+  @Override
+  public Statement apply(Statement s, Description d) {
+    return delegate.get().apply(s, d);
+  }
+
+  static <T extends TestRule> TestRuleDelegate<T> of(AtomicReference<T> delegate) {
+    return new TestRuleDelegate<T>(delegate);
+  }
+}