Add solution for quarantining flaky tests

buildtools/src/main/java/org/apache/pulsar/tests/quarantine/Quarantined.java

@@ -0,0 +1,53 @@
+/**
+ * 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.pulsar.tests.quarantine;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * {@code @Quarantined} is used to signal that the annotated test class or
+ * test method is currently <em>quarantined</em> since it is flaky.
+ *
+ * <p>Quarantined tests aren't run by default. Running quarantined tests happens
+ * by setting the {@code runQuarantinedTests} system property to true.
+ *
+ * <p>{@code @Quarantined} may optionally be declared with a {@linkplain #value
+ * reason} to document why the annotated test class or test method is quarantined.
+ *
+ * <p>When applied at the class level, all test methods within that class
+ * are all quarantined.
+ *
+ * <p>When applied at the method level, the quarantine is applied to the method.
+ *
+ * <p>This annotation will be effective when {@link QuarantinedTestNGAnnotationTransformer}
+ * class is registered as a TestNG listener.
+ */
+@Target({ElementType.TYPE, ElementType.METHOD, ElementType.CONSTRUCTOR})
+@Retention(RetentionPolicy.RUNTIME)
+@Documented
+public @interface Quarantined {
+    /**
+     * The reason this annotated test class or test method is quarantined.
+     */
+    String value() default "";
+}

buildtools/src/main/java/org/apache/pulsar/tests/quarantine/QuarantinedTestNGAnnotationTransformer.java

@@ -0,0 +1,106 @@
+/**
+ * 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.pulsar.tests.quarantine;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.IAnnotationTransformer;
+import org.testng.annotations.ITestAnnotation;
+
+/**
+ * <p>TestNG {@link IAnnotationTransformer} implementation for integrating {@link Quarantined} annotation
+ * to TestNG.
+ *
+ * <p>Flaky tests can be <em>quarantined</em> by adding a {@link Quarantined} annotation. This
+ * means that the test won't get executed by default.
+ *
+ * <p>Quarantined tests can be run by setting {@code runQuarantinedTests} system property to true.
+ * Another way to run Quarantined tests is to set {@code runOnlyQuarantinedTests} system property
+ * to true. This will result in disabling all other tests than quarantined tests.
+ *
+ */
+public class QuarantinedTestNGAnnotationTransformer implements IAnnotationTransformer {
+    private static final Logger LOG = LoggerFactory.getLogger(QuarantinedTestNGAnnotationTransformer.class);
+
+    private static final boolean DEFAULT_RUN_QUARANTINED_TESTS =
+            Boolean.parseBoolean(System.getProperty("runQuarantinedTests", "false"));
+    private static final boolean DEFAULT_RUN_ONLY_QUARANTINED_TESTS =
+            Boolean.parseBoolean(System.getProperty("runOnlyQuarantinedTests", "false"));
+
+    private final boolean runQuarantinedTests;
+    private final boolean runOnlyQuarantinedTests;
+
+    public QuarantinedTestNGAnnotationTransformer() {
+        this(DEFAULT_RUN_QUARANTINED_TESTS, DEFAULT_RUN_ONLY_QUARANTINED_TESTS);
+    }
+
+    // used for unit testing this class
+    QuarantinedTestNGAnnotationTransformer(boolean runQuarantinedTests, boolean runOnlyQuarantinedTests) {
+        this.runQuarantinedTests = runQuarantinedTests;
+        this.runOnlyQuarantinedTests = runOnlyQuarantinedTests;
+    }
+
+    @Override
+    public void transform(ITestAnnotation annotation, Class testClass, Constructor testConstructor, Method testMethod) {
+        if (!runQuarantinedTests || runOnlyQuarantinedTests) {
+            Quarantined quarantined = (Quarantined) testClass.getAnnotation(Quarantined.class);
+            if (quarantined != null) {
+                if (LOG.isDebugEnabled()) {
+                    LOG.debug("Test class {} is annotated with {} and reason '{}'",
+                            testClass.getName(), quarantined.annotationType().getName(), quarantined.value());
+                }
+            } else if (testMethod != null) {
+                quarantined = testMethod.getAnnotation(Quarantined.class);
+                if (quarantined != null) {
+                    if (LOG.isDebugEnabled()) {
+                        LOG.debug("Test method {} is annotated with {} and reason '{}'",
+                                testMethod.getName(), quarantined.annotationType().getName(), quarantined.value());
+                    }
+                }
+            } else if (testConstructor != null) {
+                quarantined = (Quarantined) testConstructor.getAnnotation(Quarantined.class);
+                if (quarantined != null) {
+                    if (LOG.isDebugEnabled()) {
+                        LOG.debug("Test constructor {} is annotated with {} and reason '{}'",
+                                testConstructor.getName(), quarantined.annotationType().getName(), quarantined.value());
+                    }
+                }
+            }
+            if (runOnlyQuarantinedTests) {
+                if (quarantined == null) {
+                    if (LOG.isDebugEnabled()) {
+                        LOG.debug("Disabling test method {} since only Quarantined methods are to be executed.",
+                                testMethod.getName());
+                    }
+                    annotation.setEnabled(false);
+                }
+            } else if (!runQuarantinedTests) {
+                if (quarantined != null) {
+                    if (LOG.isDebugEnabled()) {
+                        LOG.debug("Disabling test method {} since it is Quarantined with reason '{}'.",
+                                testMethod.getName(), quarantined.value());
+                    }
+                    annotation.setEnabled(false);
+                }
+            }
+        }
+    }
+}

buildtools/src/test/java/org/apache/pulsar/tests/quarantine/QuarantinedTestNGAnnotationTransformerTest.java

@@ -0,0 +1,200 @@
+/**
+ * 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.pulsar.tests.quarantine;
+
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+import org.testng.annotations.Test;
+import org.testng.internal.annotations.TestAnnotation;
+
+public class QuarantinedTestNGAnnotationTransformerTest {
+
+    @Quarantined
+    static class ClassWithQuarantined {
+        public void testMethod() {
+
+        }
+    }
+
+    static class ClassWithQuarantinedConstructor {
+        @Quarantined
+        public ClassWithQuarantinedConstructor() {
+
+        }
+
+        public void testMethod() {
+
+        }
+    }
+
+    static class ClassWithASingleQuarantinedMethod {
+        public ClassWithASingleQuarantinedMethod() {
+
+        }
+
+        public void testMethod() {
+
+        }
+
+        @Quarantined
+        public void testMethodQuarantined() {
+
+        }
+    }
+
+    @Test
+    public void shouldNotRunTestMethodsOnQuarantinedClass() throws NoSuchMethodException {
+        QuarantinedTestNGAnnotationTransformer quarantinedTransformer =
+                new QuarantinedTestNGAnnotationTransformer(false, false);
+
+        assertFalse(getTestAnnotation(quarantinedTransformer, ClassWithQuarantined.class,
+                "testMethod").getEnabled());
+    }
+
+    @Test
+    public void shouldRunOnlyQuarantinedTestMethodsOnQuarantinedClass() throws NoSuchMethodException {
+        // given runOnlyQuarantinedTests=true
+        QuarantinedTestNGAnnotationTransformer quarantinedTransformer =
+                new QuarantinedTestNGAnnotationTransformer(false, true);
+
+        assertTrue(getTestAnnotation(quarantinedTransformer, ClassWithQuarantined.class,
+                "testMethod").getEnabled());
+    }
+
+    @Test
+    public void shouldRunAlsoQuarantinedTestMethodsOnQuarantinedClass() throws NoSuchMethodException {
+        // given runQuarantinedTests=true
+        QuarantinedTestNGAnnotationTransformer quarantinedTransformer =
+                new QuarantinedTestNGAnnotationTransformer(true, false);
+
+        assertTrue(getTestAnnotation(quarantinedTransformer, ClassWithQuarantined.class,
+                "testMethod").getEnabled());
+    }
+
+
+    @Test
+    public void shouldNotRunQuarantinedTestMethods() throws NoSuchMethodException {
+        QuarantinedTestNGAnnotationTransformer quarantinedTransformer =
+                new QuarantinedTestNGAnnotationTransformer(false, false);
+        assertTrue(getTestAnnotation(quarantinedTransformer, ClassWithASingleQuarantinedMethod.class,
+                "testMethod").getEnabled());
+        assertFalse(getTestAnnotation(quarantinedTransformer, ClassWithASingleQuarantinedMethod.class,
+                "testMethodQuarantined").getEnabled());
+    }
+
+    @Test
+    public void shouldRunOnlyQuarantinedTestMethods() throws NoSuchMethodException {
+        // given runOnlyQuarantinedTests=true
+        QuarantinedTestNGAnnotationTransformer quarantinedTransformer =
+                new QuarantinedTestNGAnnotationTransformer(false, true);
+        assertFalse(getTestAnnotation(quarantinedTransformer, ClassWithASingleQuarantinedMethod.class,
+                "testMethod").getEnabled());
+        assertTrue(getTestAnnotation(quarantinedTransformer, ClassWithASingleQuarantinedMethod.class,
+                "testMethodQuarantined").getEnabled());
+    }
+
+    @Test
+    public void shouldRunAlsoQuarantinedTestMethods() throws NoSuchMethodException {
+        // given runQuarantinedTests=true
+        QuarantinedTestNGAnnotationTransformer quarantinedTransformer =
+                new QuarantinedTestNGAnnotationTransformer(true, false);
+        assertTrue(getTestAnnotation(quarantinedTransformer, ClassWithASingleQuarantinedMethod.class,
+                "testMethod").getEnabled());
+        assertTrue(getTestAnnotation(quarantinedTransformer, ClassWithASingleQuarantinedMethod.class,
+                "testMethodQuarantined").getEnabled());
+    }
+
+    private TestAnnotation getTestAnnotation(
+            QuarantinedTestNGAnnotationTransformer quarantinedTestNGAnnotationTransformer,
+            Class<?> clazz, String methodName)
+            throws NoSuchMethodException {
+        TestAnnotation testAnnotation = new TestAnnotation();
+        Method testMethod = clazz.getMethod(methodName);
+        quarantinedTestNGAnnotationTransformer
+                .transform(testAnnotation, clazz, null, testMethod);
+        return testAnnotation;
+    }
+
+    @Test
+    public void shouldEvaluateConstructorAnnotation() throws NoSuchMethodException {
+        QuarantinedTestNGAnnotationTransformer quarantinedTransformer =
+                new QuarantinedTestNGAnnotationTransformer(false, false);
+        assertFalse(getTestAnnotationForConstructor(quarantinedTransformer, ClassWithQuarantinedConstructor.class)
+                .getEnabled());
+    }
+
+    @Test
+    public void shouldEvaluateConstructorAnnotationWhenRunOnlyQuarantinedTests() throws NoSuchMethodException {
+        // given runOnlyQuarantinedTests=true
+        QuarantinedTestNGAnnotationTransformer quarantinedTransformer =
+                new QuarantinedTestNGAnnotationTransformer(false, true);
+        assertTrue(getTestAnnotationForConstructor(quarantinedTransformer, ClassWithQuarantinedConstructor.class)
+                .getEnabled());
+    }
+
+    @Test
+    public void shouldEvaluateConstructorAnnotationWhenRunQuarantinedTests() throws NoSuchMethodException {
+        // given runQuarantinedTests=true
+        QuarantinedTestNGAnnotationTransformer quarantinedTransformer =
+                new QuarantinedTestNGAnnotationTransformer(true, false);
+        assertTrue(getTestAnnotationForConstructor(quarantinedTransformer, ClassWithQuarantinedConstructor.class)
+                .getEnabled());
+    }
+
+    @Test
+    public void shouldEvaluateMissingConstructorAnnotation() throws NoSuchMethodException {
+        QuarantinedTestNGAnnotationTransformer quarantinedTransformer =
+                new QuarantinedTestNGAnnotationTransformer(false, false);
+        assertTrue(getTestAnnotationForConstructor(quarantinedTransformer, ClassWithASingleQuarantinedMethod.class)
+                .getEnabled());
+    }
+
+    @Test
+    public void shouldEvaluateMissingConstructorAnnotationWhenRunOnlyQuarantinedTests() throws NoSuchMethodException {
+        // given runOnlyQuarantinedTests=true
+        QuarantinedTestNGAnnotationTransformer quarantinedTransformer =
+                new QuarantinedTestNGAnnotationTransformer(false, true);
+        assertFalse(getTestAnnotationForConstructor(quarantinedTransformer, ClassWithASingleQuarantinedMethod.class)
+                .getEnabled());
+    }
+
+    @Test
+    public void shouldEvaluateMissingConstructorAnnotationWhenRunQuarantinedTests() throws NoSuchMethodException {
+        // given runQuarantinedTests=true
+        QuarantinedTestNGAnnotationTransformer quarantinedTransformer =
+                new QuarantinedTestNGAnnotationTransformer(true, false);
+        assertTrue(getTestAnnotationForConstructor(quarantinedTransformer, ClassWithASingleQuarantinedMethod.class)
+                .getEnabled());
+    }
+
+
+    private TestAnnotation getTestAnnotationForConstructor(
+            QuarantinedTestNGAnnotationTransformer quarantinedTestNGAnnotationTransformer,
+            Class<?> clazz)
+            throws NoSuchMethodException {
+        TestAnnotation testAnnotation = new TestAnnotation();
+        Constructor<?> constructor = clazz.getConstructor();
+        quarantinedTestNGAnnotationTransformer
+                .transform(testAnnotation, clazz, constructor, null);
+        return testAnnotation;
+    }
+
+}

pom.xml

@@ -1107,7 +1107,7 @@ flexible messaging model and an intuitive client API.</description>
             </property>
             <property>
               <name>listener</name>
-              <value>org.apache.pulsar.tests.PulsarTestListener,org.apache.pulsar.tests.AnnotationListener</value>
+              <value>org.apache.pulsar.tests.PulsarTestListener,org.apache.pulsar.tests.AnnotationListener,org.apache.pulsar.tests.quarantine.QuarantinedTestNGAnnotationTransformer</value>
             </property>
           </properties>
         </configuration>

tests/bc_2_0_0/pom.xml

@@ -98,7 +98,7 @@
                                 </property>
                                 <property>
                                     <name>listener</name>
-                                    <value>org.apache.pulsar.tests.PulsarTestListener,org.apache.pulsar.tests.AnnotationListener</value>
+                                    <value>org.apache.pulsar.tests.PulsarTestListener,org.apache.pulsar.tests.AnnotationListener,org.apache.pulsar.tests.quarantine.QuarantinedTestNGAnnotationTransformer</value>
                                 </property>
                             </properties>
                             <argLine>-Xmx2G -XX:MaxDirectMemorySize=8G

tests/bc_2_0_1/pom.xml

@@ -98,7 +98,7 @@
                                 </property>
                                 <property>
                                     <name>listener</name>
-                                    <value>org.apache.pulsar.tests.PulsarTestListener,org.apache.pulsar.tests.AnnotationListener</value>
+                                    <value>org.apache.pulsar.tests.PulsarTestListener,org.apache.pulsar.tests.AnnotationListener,org.apache.pulsar.tests.quarantine.QuarantinedTestNGAnnotationTransformer</value>
                                 </property>
                             </properties>
                             <argLine>-Xmx2G -XX:MaxDirectMemorySize=8G

tests/bc_2_6_0/pom.xml

@@ -105,7 +105,7 @@
                                 </property>
                                 <property>
                                     <name>listener</name>
-                                    <value>org.apache.pulsar.tests.PulsarTestListener,org.apache.pulsar.tests.AnnotationListener</value>
+                                    <value>org.apache.pulsar.tests.PulsarTestListener,org.apache.pulsar.tests.AnnotationListener,org.apache.pulsar.tests.quarantine.QuarantinedTestNGAnnotationTransformer</value>
                                 </property>
                             </properties>
                             <argLine>-Xmx2G -XX:MaxDirectMemorySize=8G