Add a check to enforce alwaysRun = true on test after-methods

core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java

@@ -134,7 +134,7 @@ MATERIALIZED_VIEW_PROPERTY_1_NAME, longProperty(MATERIALIZED_VIEW_PROPERTY_1_NAM
         queryStateMachine = stateMachine(transactionManager, createTestMetadataManager(), new AllowAllAccessControl(), testSession);
     }
 
-    @AfterMethod
+    @AfterMethod(alwaysRun = true)
     public void tearDown()
     {
         if (queryRunner != null) {

core/trino-main/src/test/java/io/trino/execution/TestCreateCatalogTask.java

@@ -76,7 +76,7 @@ public void setUp()
                 new NodeVersion("test"));
     }
 
-    @AfterMethod
+    @AfterMethod(alwaysRun = true)
     public void tearDown()
     {
         if (queryRunner != null) {

core/trino-main/src/test/java/io/trino/execution/TestDropCatalogTask.java

@@ -52,7 +52,7 @@ public void setUp()
         queryRunner.registerCatalogFactory(new TpchConnectorFactory());
     }
 
-    @AfterMethod
+    @AfterMethod(alwaysRun = true)
     public void tearDown()
     {
         if (queryRunner != null) {

plugin/trino-hive-hadoop2/src/test/java/io/trino/plugin/hive/TestHivePlugin.java

@@ -61,7 +61,7 @@ public void tearDown()
         deleteRecursively(tempDirectory, ALLOW_INSECURE);
     }
 
-    @AfterMethod
+    @AfterMethod(alwaysRun = true)
     @BeforeMethod
     public void deinitializeRubix()
     {

plugin/trino-hive/src/test/java/io/trino/plugin/hive/rubix/TestRubixCaching.java

@@ -131,7 +131,7 @@ public void setup()
         nonCachingFileSystem = getNonCachingFileSystem();
     }
 
-    @AfterMethod
+    @AfterMethod(alwaysRun = true)
     @BeforeMethod
     public void deinitializeRubix()
     {

plugin/trino-kinesis/src/test/java/io/trino/plugin/kinesis/TestMinimalFunctionality.java

@@ -163,7 +163,7 @@ public void testStreamHasData()
                 .matches("VALUES %s".formatted(count));
     }
 
-    @AfterMethod
+    @AfterMethod(alwaysRun = true)
     public void tearDown()
     {
         embeddedKinesisStream.deleteStream(streamName);

testing/trino-testing-services/src/main/java/io/trino/testng/services/ReportAfterMethodNotAlwaysRun.java

@@ -0,0 +1,104 @@
+/*
+ * 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.trino.testng.services;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
+import org.testng.IClassListener;
+import org.testng.ITestClass;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.AfterGroups;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.AfterSuite;
+import org.testng.annotations.AfterTest;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Predicate;
+
+import static com.google.common.base.Throwables.getStackTraceAsString;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static io.trino.testng.services.Listeners.reportListenerFailure;
+import static java.util.Objects.requireNonNull;
+import static java.util.function.Predicate.not;
+import static java.util.stream.Collectors.joining;
+
+public class ReportAfterMethodNotAlwaysRun
+        implements IClassListener
+{
+    private static final Set<AnnotationPredicate<?>> VIOLATIONS = ImmutableSet.of(
+            new AnnotationPredicate<>(AfterTest.class, not(AfterTest::alwaysRun)),
+            new AnnotationPredicate<>(AfterMethod.class, not(AfterMethod::alwaysRun)),
+            new AnnotationPredicate<>(AfterClass.class, not(AfterClass::alwaysRun)),
+            new AnnotationPredicate<>(AfterSuite.class, not(AfterSuite::alwaysRun)),
+            new AnnotationPredicate<>(AfterGroups.class, not(AfterGroups::alwaysRun)));
+
+    @Override
+    public void onBeforeClass(ITestClass testClass)
+    {
+        try {
+            checkHasAfterMethodsNotAlwaysRun(testClass.getRealClass());
+        }
+        catch (RuntimeException | Error e) {
+            reportListenerFailure(
+                    ReportAfterMethodNotAlwaysRun.class,
+                    "Failed to process %s:%n%s",
+                    testClass.getName(),
+                    getStackTraceAsString(e));
+        }
+    }
+
+    @VisibleForTesting
+    static void checkHasAfterMethodsNotAlwaysRun(Class<?> testClass)
+    {
+        List<Method> notAlwaysRunMethods = Arrays.stream(testClass.getMethods())
+                .filter(ReportAfterMethodNotAlwaysRun::isAfterMethodNotAlwaysRun)
+                .collect(toImmutableList());
+
+        if (!notAlwaysRunMethods.isEmpty()) {
+            throw new RuntimeException(notAlwaysRunMethods.stream()
+                    .map(Method::toString)
+                    .sorted()
+                    .collect(joining("\n", "The @AfterX methods should have the alwaysRun = true attribute to make sure that they'll run even if tests were skipped:\n", "")));
+        }
+    }
+
+    private static boolean isAfterMethodNotAlwaysRun(Method method)
+    {
+        return VIOLATIONS.stream().anyMatch(p -> p.test(method));
+    }
+
+    @Override
+    public void onAfterClass(ITestClass testClass) {}
+
+    private record AnnotationPredicate<T extends Annotation>(Class<T> annotationType, Predicate<T> predicate)
+            implements Predicate<Method>
+    {
+        AnnotationPredicate
+        {
+            requireNonNull(annotationType);
+            requireNonNull(predicate);
+        }
+
+        @Override
+        public boolean test(Method method)
+        {
+            return Optional.ofNullable(method.getAnnotation(annotationType)).filter(predicate).isPresent();
+        }
+    }
+}

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

@@ -1,4 +1,5 @@
 io.trino.testng.services.ManageTestResources
+io.trino.testng.services.ReportAfterMethodNotAlwaysRun
 io.trino.testng.services.ReportOrphanedExecutors
 io.trino.testng.services.ReportPrivateMethods
 io.trino.testng.services.ReportUnannotatedMethods