You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by dg...@apache.org on 2019/06/18 08:56:10 UTC

[ignite] branch master updated: IGNITE-11931 Rewrite @WithSystemProperty handling using JUnit rules - Fixes #6615.

This is an automated email from the ASF dual-hosted git repository.

dgovorukhin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git


The following commit(s) were added to refs/heads/master by this push:
     new 417fe7f  IGNITE-11931 Rewrite @WithSystemProperty handling using JUnit rules - Fixes #6615.
417fe7f is described below

commit 417fe7f183734f99c0786f89c31e12295d08a933
Author: Ivan Bessonov <be...@gmail.com>
AuthorDate: Tue Jun 18 11:55:21 2019 +0300

    IGNITE-11931 Rewrite @WithSystemProperty handling using JUnit rules - Fixes #6615.
    
    Signed-off-by: Dmitriy Govorukhin <dm...@gmail.com>
---
 .../junits/DelegatingJUnitStatement.java           |  64 +++++++
 .../testframework/junits/GridAbstractTest.java     | 118 +++---------
 .../testframework/junits/SystemPropertiesList.java |   6 +-
 .../testframework/junits/SystemPropertiesRule.java | 208 +++++++++++++++++++++
 .../testframework/junits/WithSystemProperty.java   |  89 ++++++++-
 5 files changed, 386 insertions(+), 99 deletions(-)

diff --git a/modules/core/src/test/java/org/apache/ignite/testframework/junits/DelegatingJUnitStatement.java b/modules/core/src/test/java/org/apache/ignite/testframework/junits/DelegatingJUnitStatement.java
new file mode 100644
index 0000000..ffefe71
--- /dev/null
+++ b/modules/core/src/test/java/org/apache/ignite/testframework/junits/DelegatingJUnitStatement.java
@@ -0,0 +1,64 @@
+/*
+ * 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.
+ */
+
+package org.apache.ignite.testframework.junits;
+
+import org.jetbrains.annotations.NotNull;
+import org.junit.runners.model.Statement;
+
+/**
+ * Utility class to convert lambdas into {@link Statement} objects.
+ *
+ * @see Statement
+ */
+public class DelegatingJUnitStatement extends Statement {
+    /** Object to delegate statements body to. */
+    private final StatementEx delegate;
+
+    /**
+     * @param delegate Object to delegate statements body to.
+     * @return {@link StatementEx} converted to {@link Statement}.
+     */
+    public static Statement wrap(@NotNull StatementEx delegate) {
+        return new DelegatingJUnitStatement(delegate);
+    }
+
+    /**
+     * @param delegate Object to delegate statements body to.
+     */
+    private DelegatingJUnitStatement(@NotNull StatementEx delegate) {
+        this.delegate = delegate;
+    }
+
+    /** {@inheritDoc} */
+    @Override public void evaluate() throws Throwable {
+        delegate.evaluate();
+    }
+
+    /**
+     * Functional version of {@link Statement} abstract class.
+     *
+     * @see Statement
+     */
+    @FunctionalInterface
+    public static interface StatementEx {
+        /**
+         * @see Statement#evaluate()
+         */
+        void evaluate() throws Throwable;
+    }
+}
diff --git a/modules/core/src/test/java/org/apache/ignite/testframework/junits/GridAbstractTest.java b/modules/core/src/test/java/org/apache/ignite/testframework/junits/GridAbstractTest.java
index 0d845b3..d93e427 100755
--- a/modules/core/src/test/java/org/apache/ignite/testframework/junits/GridAbstractTest.java
+++ b/modules/core/src/test/java/org/apache/ignite/testframework/junits/GridAbstractTest.java
@@ -32,7 +32,6 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
@@ -83,7 +82,6 @@ import org.apache.ignite.internal.util.IgniteUtils;
 import org.apache.ignite.internal.util.lang.GridAbsPredicate;
 import org.apache.ignite.internal.util.typedef.F;
 import org.apache.ignite.internal.util.typedef.G;
-import org.apache.ignite.internal.util.typedef.T2;
 import org.apache.ignite.internal.util.typedef.internal.LT;
 import org.apache.ignite.internal.util.typedef.internal.U;
 import org.apache.ignite.lang.IgniteBiTuple;
@@ -181,7 +179,9 @@ public abstract class GridAbstractTest extends JUnitAssertAware {
     protected static final String DEFAULT_CACHE_NAME = "default";
 
     /** Sustains {@link #beforeTestsStarted()} and {@link #afterTestsStopped()} methods execution.*/
-    @ClassRule public static final TestRule firstLastTestRule = new BeforeFirstAndAfterLastTestRule();
+    @ClassRule public static final TestRule firstLastTestRule = RuleChain
+        .outerRule(new SystemPropertiesRule())
+        .around(new BeforeFirstAndAfterLastTestRule());
 
     /** Manages test execution and reporting. */
     private transient TestRule runRule = (base, desc) -> new Statement() {
@@ -215,7 +215,8 @@ public abstract class GridAbstractTest extends JUnitAssertAware {
      * first.
      */
     @Rule public transient RuleChain nameAndRunRulesChain = RuleChain
-        .outerRule(nameRule)
+        .outerRule(new SystemPropertiesRule())
+        .around(nameRule)
         .around(runRule);
 
     /** */
@@ -236,12 +237,6 @@ public abstract class GridAbstractTest extends JUnitAssertAware {
     /** Lazily initialized current test method. */
     private volatile Method currTestMtd;
 
-    /** List of system properties to set when all tests in class are finished. */
-    private final List<T2<String, String>> clsSysProps = new LinkedList<>();
-
-    /** List of system properties to set when test is finished. */
-    private final List<T2<String, String>> testSysProps = new LinkedList<>();
-
     /** */
     static {
         System.setProperty(IgniteSystemProperties.IGNITE_ALLOW_ATOMIC_OPS_IN_TX, "false");
@@ -686,82 +681,6 @@ public abstract class GridAbstractTest extends JUnitAssertAware {
         }
     }
 
-    /** */
-    private void setSystemPropertiesBeforeClass() {
-        List<WithSystemProperty[]> allProps = new LinkedList<>();
-
-        for (Class<?> clazz = getClass(); clazz != GridAbstractTest.class; clazz = clazz.getSuperclass()) {
-            SystemPropertiesList clsProps = clazz.getAnnotation(SystemPropertiesList.class);
-
-            if (clsProps != null)
-                allProps.add(0, clsProps.value());
-            else {
-                WithSystemProperty clsProp = clazz.getAnnotation(WithSystemProperty.class);
-
-                if (clsProp != null)
-                    allProps.add(0, new WithSystemProperty[] {clsProp});
-            }
-        }
-
-        for (WithSystemProperty[] props : allProps) {
-            for (WithSystemProperty prop : props) {
-                String oldVal = System.setProperty(prop.key(), prop.value());
-
-                clsSysProps.add(0, new T2<>(prop.key(), oldVal));
-            }
-        }
-    }
-
-    /** */
-    private void clearSystemPropertiesAfterClass() {
-        for (T2<String, String> t2 : clsSysProps) {
-            if (t2.getValue() == null)
-                System.clearProperty(t2.getKey());
-            else
-                System.setProperty(t2.getKey(), t2.getValue());
-        }
-
-        clsSysProps.clear();
-    }
-
-    /** */
-    @Before
-    public void setSystemPropertiesBeforeTest() {
-        WithSystemProperty[] allProps = null;
-
-        SystemPropertiesList testProps = currentTestAnnotation(SystemPropertiesList.class);
-
-        if (testProps != null)
-            allProps = testProps.value();
-        else {
-            WithSystemProperty testProp = currentTestAnnotation(WithSystemProperty.class);
-
-            if (testProp != null)
-                allProps = new WithSystemProperty[] {testProp};
-        }
-
-        if (allProps != null) {
-            for (WithSystemProperty prop : allProps) {
-                String oldVal = System.setProperty(prop.key(), prop.value());
-
-                testSysProps.add(0, new T2<>(prop.key(), oldVal));
-            }
-        }
-    }
-
-    /** */
-    @After
-    public void clearSystemPropertiesAfterTest() {
-        for (T2<String, String> t2 : testSysProps) {
-            if (t2.getValue() == null)
-                System.clearProperty(t2.getKey());
-            else
-                System.setProperty(t2.getKey(), t2.getValue());
-        }
-
-        testSysProps.clear();
-    }
-
     /**
      * @return Test description.
      */
@@ -2674,21 +2593,30 @@ public abstract class GridAbstractTest extends JUnitAssertAware {
      * @param stmt Statement to execute.
      * @throws Throwable In case of failure.
      */
+    @SuppressWarnings("ThrowFromFinallyBlock")
     private void evaluateInsideFixture(Statement stmt) throws Throwable {
+        Throwable suppressed = null;
+
         try {
-            setSystemPropertiesBeforeClass();
+            beforeFirstTest();
 
-            try {
-                beforeFirstTest();
+            stmt.evaluate();
+        }
+        catch (Throwable t) {
+            suppressed = t;
 
-                stmt.evaluate();
-            }
-            finally {
-                afterLastTest();
-            }
+            throw t;
         }
         finally {
-            clearSystemPropertiesAfterClass();
+            try {
+                afterLastTest();
+            }
+            catch (Throwable t) {
+                if (suppressed != null)
+                    t.addSuppressed(suppressed);
+
+                throw t;
+            }
         }
     }
 }
diff --git a/modules/core/src/test/java/org/apache/ignite/testframework/junits/SystemPropertiesList.java b/modules/core/src/test/java/org/apache/ignite/testframework/junits/SystemPropertiesList.java
index cf43b24..2d7a062 100644
--- a/modules/core/src/test/java/org/apache/ignite/testframework/junits/SystemPropertiesList.java
+++ b/modules/core/src/test/java/org/apache/ignite/testframework/junits/SystemPropertiesList.java
@@ -18,16 +18,18 @@
 package org.apache.ignite.testframework.junits;
 
 import java.lang.annotation.ElementType;
+import java.lang.annotation.Repeatable;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 /**
- *
+ * {@link Repeatable} for the {@link WithSystemProperty}.<br/>
+ * Should not be used direclty, use multiple {@link WithSystemProperty} annotation instead.
  */
 @Retention(RetentionPolicy.RUNTIME)
 @Target({ElementType.TYPE, ElementType.METHOD})
 public @interface SystemPropertiesList {
-    /** */
+    /** Array of underlying annotations. */
     WithSystemProperty[] value();
 }
diff --git a/modules/core/src/test/java/org/apache/ignite/testframework/junits/SystemPropertiesRule.java b/modules/core/src/test/java/org/apache/ignite/testframework/junits/SystemPropertiesRule.java
new file mode 100644
index 0000000..52af7a9
--- /dev/null
+++ b/modules/core/src/test/java/org/apache/ignite/testframework/junits/SystemPropertiesRule.java
@@ -0,0 +1,208 @@
+/*
+ * 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.
+ */
+
+package org.apache.ignite.testframework.junits;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import org.apache.ignite.internal.util.typedef.T2;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runners.model.Statement;
+
+/**
+ * JUnit rule that manages usage of {@link WithSystemProperty} annotations.<br/>
+ * Can be used as both {@link Rule} and {@link ClassRule}.
+ *
+ * @see WithSystemProperty
+ * @see Rule
+ * @see ClassRule
+ */
+public class SystemPropertiesRule implements TestRule {
+    /**
+     * {@inheritDoc}
+     *
+     * @throws NoSuchMethodError If test method wasn't found for some reason.
+     */
+    @Override public Statement apply(Statement base, Description desc) {
+        Class<?> testCls = desc.getTestClass();
+
+        String testName = desc.getMethodName();
+
+        if (testName == null)
+            return classStatement(testCls, base);
+        else
+            return methodStatement(getTestMethod(testCls, testName), base);
+    }
+
+    /**
+     * Searches for public method with no parameter by the class and methods name.
+     *
+     * @param testCls Class to search method in.
+     * @param testName Method name.
+     * @return Non-null method object.
+     * @throws NoSuchMethodError If test method wasn't found.
+     */
+    private Method getTestMethod(Class<?> testCls, String testName) {
+        // Remove custom parameters from "@Parameterized" test.
+        int bracketIdx = testName.indexOf('[');
+
+        String testMtdName = bracketIdx >= 0 ? testName.substring(0, bracketIdx) : testName;
+
+        Method testMtd;
+        try {
+            testMtd = testCls.getMethod(testMtdName);
+
+        }
+        catch (NoSuchMethodException e) {
+            throw new NoSuchMethodError(S.toString("Test method wasn't found",
+                "testClass", testCls.getSimpleName(), false,
+                "methodName", testName, false,
+                "testMtdName", testMtdName, false
+            ));
+        }
+        return testMtd;
+    }
+
+    /**
+     * @return Statement that sets all required system properties before class and cleans them after.
+     */
+    private Statement classStatement(Class<?> testCls, Statement base) {
+        return DelegatingJUnitStatement.wrap(() -> {
+            List<T2<String, String>> clsSysProps = setSystemPropertiesBeforeClass(testCls);
+
+            try {
+                base.evaluate();
+            }
+            finally {
+                clearSystemProperties(clsSysProps);
+            }
+        });
+    }
+
+    /**
+     * @return Statement that sets all required system properties before test method and cleans them after.
+     */
+    private Statement methodStatement(Method testMtd, Statement base) {
+        return DelegatingJUnitStatement.wrap(() -> {
+            List<T2<String, String>> testSysProps = setSystemPropertiesBeforeTestMethod(testMtd);
+
+            try {
+                base.evaluate();
+            }
+            finally {
+                clearSystemProperties(testSysProps);
+            }
+        });
+    }
+
+    /**
+     * Set system properties before class.
+     *
+     * @param testCls Current test class.
+     * @return List of updated properties in reversed order.
+     */
+    private List<T2<String, String>> setSystemPropertiesBeforeClass(Class<?> testCls) {
+        List<WithSystemProperty[]> allProps = new ArrayList<>();
+
+        for (Class<?> cls = testCls; cls != null; cls = cls.getSuperclass()) {
+            SystemPropertiesList clsProps = cls.getAnnotation(SystemPropertiesList.class);
+
+            if (clsProps != null)
+                allProps.add(clsProps.value());
+            else {
+                WithSystemProperty clsProp = cls.getAnnotation(WithSystemProperty.class);
+
+                if (clsProp != null)
+                    allProps.add(new WithSystemProperty[] {clsProp});
+            }
+        }
+
+        Collections.reverse(allProps);
+
+        // List of system properties to set when all tests in class are finished.
+        final List<T2<String, String>> clsSysProps = new ArrayList<>();
+
+        for (WithSystemProperty[] props : allProps) {
+            for (WithSystemProperty prop : props) {
+                String oldVal = System.setProperty(prop.key(), prop.value());
+
+                clsSysProps.add(new T2<>(prop.key(), oldVal));
+            }
+        }
+
+        Collections.reverse(clsSysProps);
+
+        return clsSysProps;
+    }
+
+    /**
+     * Set system properties before test method.
+     *
+     * @param testMtd Current test method.
+     * @return List of updated properties in reversed order.
+     */
+    public List<T2<String, String>> setSystemPropertiesBeforeTestMethod(Method testMtd) {
+        WithSystemProperty[] allProps = null;
+
+        SystemPropertiesList testProps = testMtd.getAnnotation(SystemPropertiesList.class);
+
+        if (testProps != null)
+            allProps = testProps.value();
+        else {
+            WithSystemProperty testProp = testMtd.getAnnotation(WithSystemProperty.class);
+
+            if (testProp != null)
+                allProps = new WithSystemProperty[] {testProp};
+        }
+
+        // List of system properties to set when test is finished.
+        List<T2<String, String>> testSysProps = new ArrayList<>();
+
+        if (allProps != null) {
+            for (WithSystemProperty prop : allProps) {
+                String oldVal = System.setProperty(prop.key(), prop.value());
+
+                testSysProps.add(new T2<>(prop.key(), oldVal));
+            }
+        }
+
+        Collections.reverse(testSysProps);
+
+        return testSysProps;
+    }
+
+    /**
+     * Return old values of updated properties.
+     *
+     * @param sysProps List previously returned by {@link #setSystemPropertiesBeforeClass(java.lang.Class)}
+     *      or {@link #setSystemPropertiesBeforeTestMethod(Method)}.
+     */
+    private void clearSystemProperties(List<T2<String, String>> sysProps) {
+        for (T2<String, String> t2 : sysProps) {
+            if (t2.getValue() == null)
+                System.clearProperty(t2.getKey());
+            else
+                System.setProperty(t2.getKey(), t2.getValue());
+        }
+    }
+}
diff --git a/modules/core/src/test/java/org/apache/ignite/testframework/junits/WithSystemProperty.java b/modules/core/src/test/java/org/apache/ignite/testframework/junits/WithSystemProperty.java
index e163e82..0ca6cad 100644
--- a/modules/core/src/test/java/org/apache/ignite/testframework/junits/WithSystemProperty.java
+++ b/modules/core/src/test/java/org/apache/ignite/testframework/junits/WithSystemProperty.java
@@ -22,17 +22,102 @@ import java.lang.annotation.Repeatable;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Rule;
 
 /**
+ * Annotation that defines a scope with specific system property configured.<br/>
+ * <br/>
+ * Might be used on class level or on method level. Multiple annotations might be applied to the same class/method.<br/>
+ * <br/>
+ * In short, these two approaches are basically equivalent:<br/>
+ * <br/>
+ * Short:
+ * <pre>{@code  @WithSystemProperty(key = "name", value = "val")
+ *  public class SomeTest {
+ *      @ClassRule
+ *      public static final TestRule classRule = new SystemPropertiesRule();
+ *  }
+ * }</pre>
+ * Long:
+ * <pre>{@code  public class SomeTest {
+ *      private static Object oldVal;
  *
+ *      @BeforeClass
+ *      public static void beforeClass() {
+ *          oldVal = System.getProperty("name");
+ *
+ *          System.setProperty("name", "val");
+ *      }
+ *
+ *      @AfterClass
+ *      public static void afterTest() {
+ *          if (oldVal == null)
+ *              System.clearProperty("name");
+ *          else
+ *              System.setProperty("name", oldVal);
+ *      }
+ *  }
+ * }</pre>
+ *
+ * Same applies to methods with the difference that annotation translates into something like {@link Before} and
+ * {@link After}. {@link Rule} must also be used instead of {@link ClassRule} in this case:
+ * <br/><br/>
+ * <pre>{@code  public class SomeTest {
+ *      @Rule
+ *      public final TestRule testRule = new SystemPropertiesRule();
+ *
+ *      @Test
+ *      @WithSystemProperty(key = "name", value = "val")
+ *      public void test() {
+ *          // ...
+ *      }
+ *  }
+ * }</pre>
+ * is equivalent to:
+ * <pre>{@code  public class SomeTest {
+ *      @Test
+ *      public void test() {
+ *          Object oldVal = System.getProperty("name");
+ *
+ *          try {
+ *              // ...
+ *          }
+ *          finally {
+ *              if (oldVal == null)
+ *                  System.clearProperty("name");
+ *              else
+ *                  System.setProperty("name", oldVal);
+ *          }
+ *      }
+ *  }
+ * }</pre>
+ * For class level annotation it applies system properties for the whole class hierarchy (ignoring interfaces, there's
+ * no linearization implemented). More specific classes have bigger priority and set their properties last. It all
+ * starts with {@link Object} which, of course, is not annotated.<br/>
+ * <br/>
+ * Test methods do not inherit their annotations from overriden methods of super class.<br/>
+ * <br/>
+ * If there are several annotation are present on class/method then they will be applied in the same order as they
+ * appear in code. It is acheved with the help of {@link Repeatable} fuature of Java annotations -
+ * {@link SystemPropertiesList} is automatically generated in such cases. For that reason it is not recommended
+ * to use {@link SystemPropertiesList} directly.
+ *
+ * @see System#setProperty(java.lang.String, java.lang.String)
+ * @see Rule
+ * @see ClassRule
+ * @see SystemPropertiesRule
+ * @see SystemPropertiesList
  */
 @Repeatable(SystemPropertiesList.class)
 @Retention(RetentionPolicy.RUNTIME)
 @Target({ElementType.TYPE, ElementType.METHOD})
 public @interface WithSystemProperty {
-    /** */
+    /** The name of the system property. */
     String key();
 
-    /** */
+    /** The value of the system property. */
     String value();
 }