Skip to content

Commit

Permalink
Report unannotated JUnit Test annotation
Browse files Browse the repository at this point in the history
  • Loading branch information
ebyhr committed Aug 17, 2023
1 parent df16c38 commit 5a37afd
Show file tree
Hide file tree
Showing 17 changed files with 265 additions and 17 deletions.
7 changes: 7 additions & 0 deletions testing/trino-testing-services/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@
<optional>true</optional>
</dependency>

<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-api</artifactId>
<!-- it's either on classpath because anyway, or not needed at all. Let's not force the dependency on downstream when not needed. -->
<optional>true</optional>
</dependency>

<dependency>
<groupId>org.openjdk.jmh</groupId>
<artifactId>jmh-core</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/*
* 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.junit;

import com.google.common.annotations.VisibleForTesting;
import io.trino.testng.services.ReportBadTestAnnotations;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.TestInstanceFactoryContext;
import org.junit.jupiter.api.extension.TestInstancePreConstructCallback;

import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;

import static com.google.common.base.Throwables.getStackTraceAsString;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static io.trino.testing.Listeners.reportListenerFailure;
import static java.util.stream.Collectors.joining;

public class ReportBadJunitTestAnnotations
implements TestInstancePreConstructCallback
{
@Override
public void preConstructTestInstance(TestInstanceFactoryContext factoryContext, ExtensionContext context)
{
Class<?> testClass = factoryContext.getTestClass();
try {
reportBadTestAnnotations(testClass);
}
catch (RuntimeException | Error e) {
reportListenerFailure(
ReportBadTestAnnotations.class,
"Failed to process %s: \n%s",
testClass,
getStackTraceAsString(e));
}
}

private void reportBadTestAnnotations(Class<?> testClass)
{
List<Method> unannotatedTestMethods = findUnannotatedInheritedTestMethods(testClass);
if (!unannotatedTestMethods.isEmpty()) {
reportListenerFailure(
ReportBadJunitTestAnnotations.class,
"Test class %s has methods which are inherited but not explicitly annotated. Are they missing @Test?%s",
testClass.getName(),
unannotatedTestMethods.stream()
.map(Method::toString)
.collect(joining("\n\t\t", "\n\t\t", "")));
}
}

@VisibleForTesting
static List<Method> findUnannotatedInheritedTestMethods(Class<?> realClass)
{
return Arrays.stream(realClass.getMethods())
.filter(method -> method.getDeclaringClass() != Object.class)
.filter(method -> !Modifier.isStatic(method.getModifiers()))
.filter(method -> !method.isBridge())
.filter(method -> isUnannotated(method) && overriddenMethodHasTestAnnotation(method))
.collect(toImmutableList());
}

private static boolean isUnannotated(Method method)
{
return Arrays.stream(method.getAnnotations()).map(Annotation::annotationType)
.noneMatch(ReportBadJunitTestAnnotations::isJUnitAnnotation);
}

private static boolean isJUnitAnnotation(Class<? extends Annotation> clazz)
{
return clazz.getPackage().getName().startsWith("org.junit.jupiter.api");
}

private static boolean overriddenMethodHasTestAnnotation(Method method)
{
if (method.isAnnotationPresent(org.junit.jupiter.api.Test.class)) {
return true;
}

// Skip methods in Object class, e.g. toString()
if (method.getDeclaringClass() == Object.class) {
return false;
}

// The test class may override the default method of the interface
for (Class<?> interfaceClass : method.getDeclaringClass().getInterfaces()) {
Optional<Method> overridden = getOverridden(method, interfaceClass);
if (overridden.isPresent() && overridden.get().isAnnotationPresent(org.junit.jupiter.api.Test.class)) {
return true;
}
}

Class<?> superClass = method.getDeclaringClass().getSuperclass();
if (superClass == null) {
return false;
}
return getOverridden(method, superClass)
.map(ReportBadJunitTestAnnotations::overriddenMethodHasTestAnnotation)
.orElse(false);
}

private static Optional<Method> getOverridden(Method method, Class<?> base)
{
try {
// Simplistic override detection
return Optional.of(base.getMethod(method.getName(), method.getParameterTypes()));
}
catch (NoSuchMethodException ignored) {
return Optional.empty();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,26 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.trino.testng.services;
package io.trino.testing;

import com.google.common.base.Joiner;
import com.google.errorprone.annotations.FormatMethod;
import org.testng.ITestClass;
import org.testng.ITestNGListener;
import org.testng.ITestResult;

import static java.lang.String.format;

final class Listeners
public final class Listeners
{
private Listeners() {}

/**
* Print error to standard error and exit JVM.
*
* @apiNote A TestNG listener cannot throw an exception, as this are not currently properly handled by TestNG.
* @apiNote A TestNG listener and JUnit extension cannot throw an exception, as this are not currently properly handled by them.
*/
@FormatMethod
public static void reportListenerFailure(Class<? extends ITestNGListener> listenerClass, String format, Object... args)
public static void reportListenerFailure(Class<?> listenerClass, String format, Object... args)
{
System.err.println(format("FATAL: %s: ", listenerClass.getName()) + format(format, args));
System.err.println("JVM will be terminated");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

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 io.trino.testing.Listeners.reportListenerFailure;
import static java.lang.String.format;
import static java.util.stream.Collectors.joining;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
import static com.google.common.base.Throwables.getStackTraceAsString;
import static io.airlift.concurrent.Threads.daemonThreadsNamed;
import static io.airlift.units.Duration.nanosSince;
import static io.trino.testng.services.Listeners.formatTestName;
import static io.trino.testng.services.Listeners.reportListenerFailure;
import static io.trino.testing.Listeners.formatTestName;
import static io.trino.testing.Listeners.reportListenerFailure;
import static java.lang.String.format;
import static java.lang.management.ManagementFactory.getThreadMXBean;
import static java.util.concurrent.TimeUnit.MINUTES;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
import static com.google.common.base.Throwables.getStackTraceAsString;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.MoreCollectors.toOptional;
import static io.trino.testng.services.Listeners.reportListenerFailure;
import static io.trino.testing.Listeners.reportListenerFailure;
import static io.trino.testng.services.ManageTestResources.Stage.AFTER_CLASS;
import static io.trino.testng.services.ManageTestResources.Stage.BEFORE_CLASS;
import static java.lang.annotation.ElementType.FIELD;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import java.math.BigDecimal;
import java.math.RoundingMode;

import static io.trino.testng.services.Listeners.formatTestName;
import static io.trino.testing.Listeners.formatTestName;
import static java.lang.String.format;

public class ProgressLoggingListener
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

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 io.trino.testing.Listeners.reportListenerFailure;
import static java.util.Objects.requireNonNull;
import static java.util.function.Predicate.not;
import static java.util.stream.Collectors.joining;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

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 io.trino.testing.Listeners.reportListenerFailure;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.TYPE;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import org.testng.ITestClass;

import static com.google.common.base.Throwables.getStackTraceAsString;
import static io.trino.testng.services.Listeners.reportListenerFailure;
import static io.trino.testing.Listeners.reportListenerFailure;
import static io.trino.testng.services.ReportBadTestAnnotations.isTemptoClass;

public class ReportIllNamedTest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import java.util.Optional;

import static com.google.common.base.Throwables.getStackTraceAsString;
import static io.trino.testng.services.Listeners.reportListenerFailure;
import static io.trino.testing.Listeners.reportListenerFailure;

/**
* Detects test classes which are defined as inner classes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import java.lang.reflect.Method;

import static com.google.common.base.Throwables.getStackTraceAsString;
import static io.trino.testng.services.Listeners.reportListenerFailure;
import static io.trino.testing.Listeners.reportListenerFailure;
import static java.lang.String.format;

public class ReportMultiThreadedBeforeOrAfterMethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

import static com.google.common.base.Throwables.getStackTraceAsString;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static io.trino.testng.services.Listeners.reportListenerFailure;
import static io.trino.testing.Listeners.reportListenerFailure;
import static java.lang.String.format;
import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

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 io.trino.testing.Listeners.reportListenerFailure;
import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import static java.util.stream.Collectors.joining;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
io.trino.junit.ReportBadJunitTestAnnotations
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
junit.jupiter.extensions.autodetection.enabled=true
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
* 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.junit;

import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;

import java.lang.reflect.Method;

import static io.trino.junit.ReportBadJunitTestAnnotations.findUnannotatedInheritedTestMethods;
import static org.assertj.core.api.Assertions.assertThat;

public class TestReportBadJunitTestAnnotations
{
@Test
public void testTest()
{
assertThat(findUnannotatedInheritedTestMethods(TestingTest.class))
.isEmpty();
assertThat(findUnannotatedInheritedTestMethods(TestingTestWithProxy.class))
.isEmpty();
assertThat(findUnannotatedInheritedTestMethods(TestingBeforeAfterAnnotations.class))
.isEmpty();
assertThat(findUnannotatedInheritedTestMethods(TestingTestWithoutTestAnnotation.class))
.isEmpty();
}

@Test
public void testTestWithoutTestAnnotation()
{
assertThat(findUnannotatedInheritedTestMethods(TestingTestWithoutAnnotation.class))
.extracting(Method::getName)
.containsExactly("testInInterface");
}

private static class TestingTest
implements TestingInterfaceWithTest
{
@Test
public void test() {}
}

private static class TestingTestWithoutAnnotation
implements TestingInterfaceWithTest
{
@Override
public void testInInterface()
{
TestingInterfaceWithTest.super.testInInterface();
}
}

private static class TestingTestWithProxy
extends TestingInterfaceWithTestProxy
{
@Test
public void test() {}
}

private static class TestingTestWithoutTestAnnotation
implements TestingInterface
{
public void testWithMissingTestAnnotation() {}

@Override
public String toString()
{
return "test override";
}
}

private static class TestingInterfaceWithTestProxy
implements TestingInterfaceWithTest {}

private interface TestingInterfaceWithTest
{
@Test
default void testInInterface() {}
}

private interface TestingInterface
{
default void methodInInterface() {}
}

private static class TestingBeforeAfterAnnotations
extends BaseTest {}

private static class BaseTest
{
@BeforeAll
@BeforeClass
public final void initialize() {}

@AfterAll
@AfterClass(alwaysRun = true)
public final void destroy() {}
}
}

0 comments on commit 5a37afd

Please sign in to comment.