Forbid @BeforeMethod, @AfterMethod in multi-threaded tests

Existence of `@BeforeMethod` or `@AfterMethod` most often indicates a
shared mutable state that needs to be e.g. reset. This means the class
should be marked `@Test(singleThreaded=true)`.

presto-testng-services/src/main/java/io/prestosql/testng/services/ReportMultiThreadedBeforeOrAfterMethod.java

@@ -0,0 +1,68 @@
+/*
+ * Licensed 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 io.prestosql.testng.services;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.testng.IClassListener;
+import org.testng.ITestClass;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import java.lang.reflect.Method;
+
+import static com.google.common.base.Throwables.getStackTraceAsString;
+import static io.prestosql.testng.services.Listeners.reportListenerFailure;
+import static java.lang.String.format;
+
+public class ReportMultiThreadedBeforeOrAfterMethod
+        implements IClassListener
+{
+    @Override
+    public void onBeforeClass(ITestClass testClass)
+    {
+        try {
+            reportMultiThreadedBeforeOrAfterMethod(testClass.getRealClass());
+        }
+        catch (RuntimeException | Error e) {
+            reportListenerFailure(
+                    ReportMultiThreadedBeforeOrAfterMethod.class,
+                    "Failed to process %s: \n%s",
+                    testClass,
+                    getStackTraceAsString(e));
+        }
+    }
+
+    @VisibleForTesting
+    static void reportMultiThreadedBeforeOrAfterMethod(Class<?> testClass)
+    {
+        Test testAnnotation = testClass.getAnnotation(Test.class);
+        if (testAnnotation != null && testAnnotation.singleThreaded()) {
+            return;
+        }
+
+        Method[] methods = testClass.getMethods();
+        for (Method method : methods) {
+            if (method.getAnnotation(BeforeMethod.class) != null || method.getAnnotation(AfterMethod.class) != null) {
+                throw new RuntimeException(format(
+                        "Test class %s should be annotated as @Test(singleThreaded=true), if it contains mutable state as indicated by %s",
+                        testClass.getName(),
+                        method));
+            }
+        }
+    }
+
+    @Override
+    public void onAfterClass(ITestClass iTestClass) {}
+}

presto-testng-services/src/main/resources/META-INF/services/org.testng.ITestNGListener

@@ -1,5 +1,6 @@
 io.prestosql.testng.services.ReportOrphanedExecutors
 io.prestosql.testng.services.ReportUnannotatedMethods
 io.prestosql.testng.services.ReportIllNamedTest
+io.prestosql.testng.services.ReportMultiThreadedBeforeOrAfterMethod
 io.prestosql.testng.services.LogTestDurationListener
 io.prestosql.testng.services.RetryAnnotationTransformer

presto-testng-services/src/test/java/io/prestosql/testng/services/TestReportMultiThreadedBeforeOrAfterMethod.java

@@ -0,0 +1,69 @@
+/*
+ * Licensed 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 io.prestosql.testng.services;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import static io.prestosql.testng.services.ReportMultiThreadedBeforeOrAfterMethod.reportMultiThreadedBeforeOrAfterMethod;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+public class TestReportMultiThreadedBeforeOrAfterMethod
+{
+    @Test
+    public void test()
+    {
+        // no @BeforeMethod
+        reportMultiThreadedBeforeOrAfterMethod(ClassWithoutBeforeMethod.class);
+
+        // @BeforeMethod but not @Test(singleThreaded)
+        assertThatThrownBy(() -> reportMultiThreadedBeforeOrAfterMethod(ClassWithBeforeMethod.class))
+                .hasMessageMatching("Test class \\S*\\Q.TestReportMultiThreadedBeforeOrAfterMethod$ClassWithBeforeMethod should be annotated as @Test(singleThreaded=true)\\E, " +
+                        "if it contains mutable state as indicated by public void \\S*\\Q.TestReportMultiThreadedBeforeOrAfterMethod$ClassWithBeforeMethod.someBeforeMethod()");
+
+        // @BeforeMethod and @Test(singleThreaded)
+        reportMultiThreadedBeforeOrAfterMethod(SingleThreadedClassWithBeforeMethod.class);
+
+        // inherited @BeforeMethod but not @Test(singleThreaded)
+        assertThatThrownBy(() -> reportMultiThreadedBeforeOrAfterMethod(ClassWithInheritedBeforeMethod.class))
+                .hasMessageMatching("Test class \\S*\\Q.TestReportMultiThreadedBeforeOrAfterMethod$ClassWithInheritedBeforeMethod should be annotated as @Test(singleThreaded=true)\\E, " +
+                        "if it contains mutable state as indicated by public void \\S*\\Q.TestReportMultiThreadedBeforeOrAfterMethod$ClassWithBeforeMethod.someBeforeMethod()");
+
+        // inherited @BeforeMethod and "inherited" @Test(singleThreaded) (this does not get propagated to child class yet, see https://github.com/cbeust/testng/issues/144)
+        assertThatThrownBy(() -> reportMultiThreadedBeforeOrAfterMethod(ClassExtendingSingleThreadedClassWithBeforeMethod.class))
+                .hasMessageMatching("Test class \\S*\\Q.TestReportMultiThreadedBeforeOrAfterMethod$ClassExtendingSingleThreadedClassWithBeforeMethod should be annotated as @Test(singleThreaded=true)\\E, " +
+                        "if it contains mutable state as indicated by public void \\S*\\Q.TestReportMultiThreadedBeforeOrAfterMethod$SingleThreadedClassWithBeforeMethod.someBeforeMethod()");
+    }
+
+    public static class ClassWithoutBeforeMethod {}
+
+    public static class ClassWithBeforeMethod
+    {
+        @BeforeMethod
+        public void someBeforeMethod() {}
+    }
+
+    @Test(singleThreaded = true)
+    public static class SingleThreadedClassWithBeforeMethod
+    {
+        @BeforeMethod
+        public void someBeforeMethod() {}
+    }
+
+    public static class ClassWithInheritedBeforeMethod
+            extends ClassWithBeforeMethod {}
+
+    public static class ClassExtendingSingleThreadedClassWithBeforeMethod
+            extends SingleThreadedClassWithBeforeMethod {}
+}