perf-test-support: @Before/AfterMethod/Class support is now in classes

Mixin interfaces are just marker interfaces, explanation there.

(cherry picked from commit ff6cbad)

infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/AbstractSchemaPerformanceTest.java

@@ -14,7 +14,6 @@
 import org.javasimon.Split;
 import org.javasimon.Stopwatch;
 import org.jetbrains.annotations.NotNull;
-import org.testng.annotations.BeforeClass;
 import org.testng.annotations.BeforeSuite;
 import org.xml.sax.SAXException;
 
@@ -58,12 +57,6 @@ public void setup() throws SchemaException, SAXException, IOException {
         assert !InternalsConfig.isConsistencyChecks();
     }
 
-    @BeforeClass
-    @Override
-    public void initTestMonitor() {
-        PerformanceTestClassMixin.super.initTestMonitor();
-    }
-
     protected void measure(String label, String note, CheckedProducer<?> producer) throws CommonException, IOException {
         measure(label, note, producer, DEFAULT_EXECUTION, DEFAULT_REPEATS);
     }

infra/test-util/src/main/java/com/evolveum/midpoint/test/util/AbstractSpringTest.java

@@ -7,6 +7,7 @@
 package com.evolveum.midpoint.test.util;
 
 import java.lang.reflect.Field;
+import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 
 import org.jetbrains.annotations.Nullable;
@@ -34,6 +35,7 @@ public abstract class AbstractSpringTest extends AbstractTestNGSpringContextTest
      */
     protected final Trace logger = TraceManager.getTrace(getClass());
 
+    // region perf-test support
     private TestMonitor testMonitor;
 
     /** Called only by tests that need it, implements performance mixin interface. */
@@ -52,23 +54,49 @@ public TestMonitor testMonitor() {
         return testMonitor;
     }
 
-    @BeforeClass
-    public void displayTestClassTitle() {
-        displayTestTitle("Starting TEST CLASS: " + getClass().getName());
+    // see the comment in PerformanceTestMethodMixin for explanation
+    @BeforeMethod
+    public void initTestMethodMonitor() {
+        if (this instanceof PerformanceTestMethodMixin) {
+            createTestMonitor();
+        }
     }
 
-    @AfterClass
-    public void displayTestClassFooter() {
-        displayTestFooter("Finishing with TEST CLASS: " + getClass().getName());
+    // see the comment in PerformanceTestMethodMixin for explanation
+    @AfterMethod
+    public void dumpMethodReport(Method method) {
+        if (this instanceof PerformanceTestMethodMixin) {
+            ((PerformanceTestMethodMixin) this).dumpReport(
+                    getClass().getSimpleName() + "#" + method.getName());
+        }
+    }
+
+    // see the comment in PerformanceTestClassMixin for explanation
+    @BeforeClass
+    public void initTestClassMonitor() {
+        if (this instanceof PerformanceTestClassMixin) {
+            createTestMonitor();
+        }
     }
 
     // see the comment in PerformanceTestClassMixin for explanation
     @AfterClass
-    public void dumpReport() {
+    public void dumpClassReport() {
         if (this instanceof PerformanceTestClassMixin) {
             ((PerformanceTestClassMixin) this).dumpReport(getClass().getSimpleName());
         }
     }
+    // endregion
+
+    @BeforeClass
+    public void displayTestClassTitle() {
+        displayTestTitle("Starting TEST CLASS: " + getClass().getName());
+    }
+
+    @AfterClass
+    public void displayTestClassFooter() {
+        displayTestFooter("Finishing with TEST CLASS: " + getClass().getName());
+    }
 
     @BeforeMethod
     public void startTestContext(ITestResult testResult) throws Exception {
@@ -107,7 +135,7 @@ public MidpointTestContext getTestContext() {
      * Note that this does not work for components injected through constructor into
      * final fields - if we need this cleanup, make the field non-final.
      */
-    @AfterClass(alwaysRun = true, dependsOnMethods = "dumpReport")
+    @AfterClass(alwaysRun = true, dependsOnMethods = "dumpClassReport")
     protected void clearClassFields() throws Exception {
         logger.trace("Clearing all fields for test class {}", getClass().getName());
         clearClassFields(this, getClass());

tools/test-ng/src/main/java/com/evolveum/midpoint/tools/testng/AbstractUnitTest.java

@@ -6,6 +6,8 @@
  */
 package com.evolveum.midpoint.tools.testng;
 
+import java.lang.reflect.Method;
+
 import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -23,6 +25,7 @@ public abstract class AbstractUnitTest implements MidpointTestMixin {
 
     protected final Logger logger = LoggerFactory.getLogger(getClass());
 
+    // region perf-test support
     private TestMonitor testMonitor;
 
     /** Called only by tests that need it, implements performance mixin interface. */
@@ -41,23 +44,49 @@ public TestMonitor testMonitor() {
         return testMonitor;
     }
 
-    @BeforeClass
-    public void displayTestClassTitle() {
-        displayTestTitle("Starting TEST CLASS: " + getClass().getName());
+    // see the comment in PerformanceTestMethodMixin for explanation
+    @BeforeMethod
+    public void initTestMethodMonitor() {
+        if (this instanceof PerformanceTestMethodMixin) {
+            createTestMonitor();
+        }
     }
 
-    @AfterClass
-    public void displayTestClassFooter() {
-        displayTestFooter("Finishing with TEST CLASS: " + getClass().getName());
+    // see the comment in PerformanceTestMethodMixin for explanation
+    @AfterMethod
+    public void dumpMethodReport(Method method) {
+        if (this instanceof PerformanceTestMethodMixin) {
+            ((PerformanceTestMethodMixin) this).dumpReport(
+                    getClass().getSimpleName() + "#" + method.getName());
+        }
+    }
+
+    // see the comment in PerformanceTestClassMixin for explanation
+    @BeforeClass
+    public void initTestClassMonitor() {
+        if (this instanceof PerformanceTestClassMixin) {
+            createTestMonitor();
+        }
     }
 
     // see the comment in PerformanceTestClassMixin for explanation
     @AfterClass
-    public void dumpReport() {
+    public void dumpClassReport() {
         if (this instanceof PerformanceTestClassMixin) {
             ((PerformanceTestClassMixin) this).dumpReport(getClass().getSimpleName());
         }
     }
+    // endregion
+
+    @BeforeClass
+    public void displayTestClassTitle() {
+        displayTestTitle("Starting TEST CLASS: " + getClass().getName());
+    }
+
+    @AfterClass
+    public void displayTestClassFooter() {
+        displayTestFooter("Finishing with TEST CLASS: " + getClass().getName());
+    }
 
     @BeforeMethod
     public void startTestContext(ITestResult testResult) {

tools/test-ng/src/main/java/com/evolveum/midpoint/tools/testng/PerformanceTestClassMixin.java

@@ -6,25 +6,25 @@
  */
 package com.evolveum.midpoint.tools.testng;
 
-import org.testng.annotations.BeforeClass;
-
 /**
  * Mixin supporting work with {@link TestMonitor} at the class-scope level
  * (one test report for all the methods in the test class).
  * Details of setting of {@link TestMonitor} is up to the class, methods from
  * {@link PerformanceTestCommonMixin} must be implemented.
+ *
+ * [NOTE]
+ * ====
+ * Actual `@Before/AfterClass` methods are implemented in `AbstractUnitTest`
+ * and `AbstractSpringTest` using `instanceof` check for two reasons:
+ *
+ * * If `@AfterClass` is in interface it is executed after all lifecycle methods from the class
+ * hierarchy - which may happen after the Spring context is destroyed (for integration tests).
+ * * If mixin interface is on the abstract class the lifecycle methods *are not called at all*
+ * in the test subclasses, which really sucks.
+ *
+ * So currently this is only marker interface used by lifecycle methods in our two top-level
+ * classes (unit/Spring) and everything works fine.
+ * ====
  */
 public interface PerformanceTestClassMixin extends PerformanceTestCommonMixin {
-
-    @BeforeClass
-    default void initTestMonitor() {
-        createTestMonitor();
-    }
-
-    /*
-     * @AfterClass dumpReport() implemented in AbstractUnitTest and AbstractSpringTest.
-     * After-class method is implemented in both concrete classes with `instanceof` check,
-     * because if implemented here the method is called too late.
-     * This is not a problem for before-class method, so it stays here.
-     */
 }

tools/test-ng/src/main/java/com/evolveum/midpoint/tools/testng/PerformanceTestMethodMixin.java

@@ -1,31 +1,30 @@
 /*
- * Copyright (C) 2010-2020 Evolveum and contributors
+ * Copyright (C) 2010-2021 Evolveum and contributors
  *
  * This work is dual-licensed under the Apache License 2.0
  * and European Union Public License. See LICENSE file for details.
  */
 package com.evolveum.midpoint.tools.testng;
 
-import java.lang.reflect.Method;
-
-import org.testng.annotations.AfterMethod;
-import org.testng.annotations.BeforeMethod;
-
 /**
  * Mixin supporting work with {@link TestMonitor} at the method-scope level
  * (one test report for each test method).
  * Details of setting of {@link TestMonitor} is up to the class, methods from
  * {@link PerformanceTestCommonMixin} must be implemented.
+ *
+ * [NOTE]
+ * ====
+ * Actual `@Before/AfterMethod` methods are implemented in `AbstractUnitTest`
+ * and `AbstractSpringTest` using `instanceof` check for two reasons:
+ *
+ * * If mixin interface is on the abstract class the lifecycle methods *are not called at all*
+ * in the test subclasses, which really sucks.
+ * * To use the same strategy as in {@link PerformanceTestClassMixin} which actually has another
+ * good reason to do this (issue that does not affect the method lifecycle that much).
+ *
+ * So currently this is only marker interface used by lifecycle methods in our two top-level
+ * classes (unit/Spring) and everything works fine.
+ * ====
  */
 public interface PerformanceTestMethodMixin extends PerformanceTestCommonMixin {
-
-    @BeforeMethod
-    default void initTestMonitor() {
-        createTestMonitor();
-    }
-
-    @AfterMethod
-    default void dumpReport(Method method) {
-        dumpReport(getClass().getSimpleName() + "#" + method.getName());
-    }
 }