GEODE-4181: Add JUnit 5 Support (apache#6911)

Added JUnit 5 support to all Geode projects that use `geode-junit`.

This is a re-do of PR 6740 with the addition of a new parameterized test
runner. See the GEODE PARAMS RUNNER section (below) for details.

STANDARD TEST TASKS

Updated **./gradle/test.gradle** to configure these standard test tasks
to use JUnit Platform to run tests:
- `test` and `repeatUnitTest`
- `acceptanceTest` and `repeatAcceptanceTest`
- `distributedTest` and `repeatDistributedTest`
- `integrationTest` and `repeatIntegrationTest`
- `performanceTest`
- `uiTest`
- `upgradeTest` and `repeatUpgradeTest`

STANDARD TEST MODULES

Updated **./geode-junit/build.gradle**:
- Added `junit-jupiter-api` and `junit-jupiter-params` as API
  dependencies.
- Added `junit-jupiter-engine` and `junit-vintage-engine` as
  implementation dependencies.

These changes add JUnit 5 support to any source set that depends on
`geode-junit`, either directly or via `geode-dunit`.

OTHER PROJECTS

Added `junit-vintage-engine` dependency directly to each project that
runs tests without `geode-junit` or `geode-dunit`:
- `geode-common`
- `geode-concurrency-test`
- `geode-jmh`
- `geode-modules`
- `geode-rebalancer`
- `static-analysis:pmd-rules`

These changes **do not** add JUnit 5 support to these projects.
Developers who want JUnit 5 support in these projects can declare
dependencies on `junit-jupiter-api`, `junit-jupiter-params`, and
`junit-jupiter-engine`.

SPECIFIC TESTS

Change `ConcurrencyRuleTest` to expect the exception types and exception
messages thrown by AssertJ when `opentest4j` is on the classpath.

GEODE PARAMS RUNNER

**Problem:** Geode commonly uses `JUnitParamsRunner` to run
parameterized tests. This runner is incompatible with JUnit Vintage, the
test engine that JUnit 5 uses to run JUnit 4 tests. Specifically:
- `JUnitParamsRunner` discards `@Category` annotations when describing
  parameterized tests. JUnit Vintage needs this information to filter
  tests by category.
- When JUnit Vintage asks `JUnitParamsRunner` to exclude a parameterized
  test by name, the runner applies the filter not to the individual
  tests with parameterizations, but to the suite that the runner created
  to contain the parameterized tests. The filter fails to exclude
  parameterized tests.

As a result of these issues, Geode's Windows Gfsh distributed test CI
job ends up running hundreds of unwanted tests. Numerous of these fail
on Windows.

**Solution:** Introduce `GeodeParamsRunner`, which extends and adapts
`JUnitParamsRunner` to be compatible with JUnit Vintage.

**Caveat:** When `GeodeParamsRunner` is asked to exclude a specific
parameterization of a test method, it excludes *all* parameterizations
of that test method. This makes it impossible to run a single
parameterization of a given test method, or to exclude a single
parameterization. This quirk does not affect any CI jobs, but may
confuse a developer who tries to use `--tests` to run a single
parameterization.

boms/geode-all-bom/src/test/resources/expected-pom.xml

@@ -582,6 +582,26 @@
         <artifactId>hamcrest</artifactId>
         <version>2.2</version>
       </dependency>
+      <dependency>
+        <groupId>org.junit.jupiter</groupId>
+        <artifactId>junit-jupiter-api</artifactId>
+        <version>5.7.2</version>
+      </dependency>
+      <dependency>
+        <groupId>org.junit.jupiter</groupId>
+        <artifactId>junit-jupiter-params</artifactId>
+        <version>5.7.2</version>
+      </dependency>
+      <dependency>
+        <groupId>org.junit.jupiter</groupId>
+        <artifactId>junit-jupiter-engine</artifactId>
+        <version>5.7.2</version>
+      </dependency>
+      <dependency>
+        <groupId>org.junit.vintage</groupId>
+        <artifactId>junit-vintage-engine</artifactId>
+        <version>5.7.2</version>
+      </dependency>
       <dependency>
         <groupId>org.seleniumhq.selenium</groupId>
         <artifactId>selenium-api</artifactId>

buildSrc/src/main/groovy/org/apache/geode/gradle/plugins/DependencyConstraints.groovy

@@ -65,6 +65,7 @@ class DependencyConstraints implements Plugin<Project> {
 
     // These versions are referenced in test.gradle, which is aggressively injected into all projects.
     deps.put("junit.version", "4.13.2")
+    deps.put("junit-jupiter.version", "5.7.2")
     deps.put("cglib.version", "3.3.0")
     return deps
   }
@@ -221,6 +222,16 @@ class DependencyConstraints implements Plugin<Project> {
       entry('hamcrest')
     }
 
+    dependencySet(group: 'org.junit.jupiter', version: get('junit-jupiter.version')) {
+      entry('junit-jupiter-api')
+      entry('junit-jupiter-params')
+      entry('junit-jupiter-engine')
+    }
+
+    dependencySet(group: 'org.junit.vintage', version: get('junit-jupiter.version')) {
+      entry('junit-vintage-engine')
+    }
+
     dependencySet(group: 'org.seleniumhq.selenium', version: '3.141.59') {
       entry('selenium-api')
       entry('selenium-chrome-driver')

extensions/geode-modules/build.gradle

@@ -41,6 +41,7 @@ dependencies {
   // test
   testImplementation('org.apache.bcel:bcel')
   testImplementation('junit:junit')
+  testRuntimeOnly('org.junit.vintage:junit-vintage-engine')
   testImplementation('org.assertj:assertj-core')
   testImplementation('org.mockito:mockito-core')
   testImplementation('org.apache.tomcat:catalina-ha:' + DependencyConstraints.get('tomcat6.version'))

geode-apis-compatible-with-redis/build.gradle

@@ -28,6 +28,9 @@ facets {
     includeInCheckLifecycle = false
   }
 }
+commonTest {
+  useJUnitPlatform()
+}
 
 dependencies {
   api(platform(project(':boms:geode-all-bom')))

geode-common/build.gradle

@@ -28,11 +28,13 @@ dependencies {
 
   // test
   testImplementation('junit:junit')
+  testRuntimeOnly('org.junit.vintage:junit-vintage-engine')
   testImplementation('org.assertj:assertj-core')
   testImplementation('org.mockito:mockito-core')
 
 
   // jmhTest
   jmhTestImplementation('junit:junit')
+  jmhTestRuntimeOnly('org.junit.vintage:junit-vintage-engine')
   jmhTestImplementation('org.assertj:assertj-core')
 }

geode-concurrency-test/build.gradle

@@ -25,6 +25,7 @@ dependencies {
   implementation('junit:junit')
   implementation('org.apache.logging.log4j:log4j-api')
   integrationTestImplementation('org.assertj:assertj-core')
+  integrationTestRuntimeOnly('org.junit.vintage:junit-vintage-engine')
 }
 
 integrationTest {

geode-dunit/src/main/java/org/apache/geode/test/dunit/internal/ProcessManager.java

@@ -338,6 +338,7 @@ private Path createPathingJar(String vmName, String classPath) throws IOExceptio
     List<String> originalClassPathEntries = Arrays.asList(classPath.split(File.pathSeparator));
     String classPathAttributeValue = originalClassPathEntries.stream()
         .map(Paths::get)
+        .filter(Files::exists)
         .map(currentWorkingDir::relativize) // Entries must be relative to pathing jar's dir
         .map(p -> Files.isDirectory(p) ? p + "/" : p.toString()) // Dir entries must end with /
         .map(s -> s.replaceAll("\\\\", "/")) // Separator must be /

geode-jmh/build.gradle

@@ -27,6 +27,7 @@ dependencies {
   api('org.openjdk.jmh:jmh-core')
 
   testImplementation('junit:junit')
+  testRuntimeOnly('org.junit.vintage:junit-vintage-engine')
   testImplementation('org.assertj:assertj-core')
   testImplementation('org.mockito:mockito-core')
 }

geode-junit/build.gradle

@@ -38,6 +38,8 @@ dependencies {
   api('junit:junit') {
     exclude module: 'hamcrest'
   }
+  api('org.junit.jupiter:junit-jupiter-api')
+  api('org.junit.jupiter:junit-jupiter-params')
   api('org.assertj:assertj-core')
   api('org.mockito:mockito-core')
 
@@ -60,6 +62,9 @@ dependencies {
 
   api('org.skyscreamer:jsonassert')
 
+  implementation('org.junit.jupiter:junit-jupiter-engine')
+  implementation('org.junit.vintage:junit-vintage-engine')
+
   implementation('pl.pragmatists:JUnitParams')
 
   integrationTestImplementation(project(':geode-core'))

geode-junit/src/main/java/org/apache/geode/test/junit/runners/GeodeParamsRunner.java

@@ -14,11 +14,154 @@
  */
 package org.apache.geode.test.junit.runners;
 
+import static java.util.Objects.isNull;
+import static org.junit.runner.Description.createTestDescription;
+
+import java.lang.reflect.Method;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
 import junitparams.JUnitParamsRunner;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.Description;
+import org.junit.runner.manipulation.Filter;
+import org.junit.runner.manipulation.NoTestsRemainException;
+import org.junit.runners.model.FrameworkMethod;
 import org.junit.runners.model.InitializationError;
 
+/**
+ * Extends and adapts {@link JUnitParamsRunner} to be compatible with the way JUnit Vintage filters
+ * tests by {@link Category}. The differences from {@code JUnitParamsRunner} are:
+ * <ul>
+ * <li>When {@code GeodeParamsRunner} describes a test class, each parameterized test description
+ * includes the {@code Category} annotation (if any) from the corresponding test method.</li>
+ * <li>When Junit Vintage applies a filter, if the filter would exclude a test with parameters,
+ * {@code GeodeParamsRunner} excludes <em>every</em> parameterization of the test method.
+ * </li>
+ * </ul>
+ * <p>
+ * <strong>Problem:</strong> {@code GeodeParamsRunner}'s filtering behavior makes it impossible
+ * to execute a subset of the parameterizations of a single test method. But because all
+ * parameterizations of a method share the same {@code Category} annotation, this behavior allows
+ * JUnit Vintage to filter tests by category. A more nuanced fix would require a significant
+ * rewrite of {@code JUnitParamsRunner}.
+ */
 public class GeodeParamsRunner extends JUnitParamsRunner {
+  private static final String EXCLUDE_FILTER_PREFIX = "exclude ";
+  private final Map<String, FrameworkMethod> testNameToFrameworkMethod = new HashMap<>();
+
   public GeodeParamsRunner(Class<?> testClass) throws InitializationError {
     super(testClass);
   }
+
+  /**
+   * Overrides {@link JUnitParamsRunner#describeMethod(FrameworkMethod)} to include a parameterized
+   * test method's {@code Category} annotation (if any) in the descriptions of the method's tests.
+   */
+  @Override
+  public Description describeMethod(FrameworkMethod method) {
+    Description description = super.describeMethod(method);
+    if (description.isTest()) {
+      return description;
+    }
+    recordTestNamesToFrameworkMethod(description.getChildren(), method);
+    return vintageCompatibleSuiteDescription(description, method.getMethod());
+  }
+
+  /**
+   * Overrides {@link JUnitParamsRunner#filter(Filter)} to exclude all of a parameterized
+   * method's tests if the filter would exclude any of the method's tests.
+   */
+  @Override
+  public void filter(Filter filter) throws NoTestsRemainException {
+    System.out.printf("DHE: filter(%s)%n", filter.describe());
+    super.filter(vintageCompatibleFilter(filter, testNameToFrameworkMethod));
+  }
+
+  @Override
+  public String toString() {
+    return getClass().getSimpleName() + "(" + getTestClass().getJavaClass().getSimpleName() + ")";
+  }
+
+  private void recordTestNamesToFrameworkMethod(Collection<Description> testDescriptions,
+      FrameworkMethod method) {
+    testDescriptions.stream()
+        .map(Description::getDisplayName)
+        .forEach(testName -> testNameToFrameworkMethod.put(testName, method));
+  }
+
+  private String displayNameOf(FrameworkMethod method) {
+    return createTestDescription(getTestClass().getJavaClass(), method.getName()).getDisplayName();
+  }
+
+  /**
+   * Adapts a filter to exclude all of a parameterized test method's tests if the filter would
+   * exclude any of the method's tests.
+   */
+  public Filter vintageCompatibleFilter(
+      Filter originalFilter, Map<String, FrameworkMethod> parameterizedTestMap) {
+    String filterDescription = originalFilter.describe();
+    if (!filterDescription.startsWith(EXCLUDE_FILTER_PREFIX)) {
+      // The filter is not an exclude filter. JUnitParamsRunner applies it correctly.
+      return originalFilter;
+    }
+    String excludedTestName = filterDescription.substring(EXCLUDE_FILTER_PREFIX.length());
+    FrameworkMethod parameterizedMethod = parameterizedTestMap.get(excludedTestName);
+    if (isNull(parameterizedMethod)) {
+      // The filter excludes a non-parameterized test. JUnitParamsRunner applies it correctly.
+      return originalFilter;
+    }
+    return new ExcludeByDisplayName(displayNameOf(parameterizedMethod));
+  }
+
+  /**
+   * Adapts a suite description to add the given method's {@code Category} annotation (if any) to
+   * the descriptions of the suite's tests.
+   */
+  private static Description vintageCompatibleSuiteDescription(
+      Description originalDescription, Method method) {
+    Category categories = method.getAnnotation(Category.class);
+    if (isNull(categories)) {
+      return originalDescription;
+    }
+    Description compatibleDescription = originalDescription.childlessCopy();
+    originalDescription.getChildren().stream()
+        .map(testDescription -> vintageCompatibleTestDescription(testDescription, categories))
+        .forEach(compatibleDescription::addChild);
+    return compatibleDescription;
+  }
+
+  /**
+   * Adapts a test description to add the given {@link Category} annotation.
+   */
+  private static Description vintageCompatibleTestDescription(
+      Description originalDescription, Category categories) {
+    Class<?> testClass = originalDescription.getTestClass();
+    String displayName = originalDescription.getMethodName();
+    return createTestDescription(testClass, displayName, categories);
+  }
+
+  /**
+   * Filter descriptions by display name.
+   */
+  private static class ExcludeByDisplayName extends Filter {
+    private final String displayNameToExclude;
+
+    public ExcludeByDisplayName(String displayNameToExclude) {
+      this.displayNameToExclude = displayNameToExclude;
+    }
+
+    @Override
+    public boolean shouldRun(Description description) {
+      boolean shouldRun = !description.getDisplayName().equals(displayNameToExclude);
+      System.out.printf("DHE: %s shouldRun(%s): %s%n", describe(), description, shouldRun);
+      return shouldRun;
+    }
+
+    @Override
+    public String describe() {
+      return "exclude display name " + displayNameToExclude;
+    }
+  }
 }

geode-junit/src/test/java/org/apache/geode/test/junit/rules/ConcurrencyRuleTest.java

@@ -32,11 +32,11 @@
 
 import junitparams.Parameters;
 import org.junit.Before;
-import org.junit.ComparisonFailure;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.model.MultipleFailureException;
+import org.opentest4j.AssertionFailedError;
 
 import org.apache.geode.test.junit.runners.GeodeParamsRunner;
 
@@ -399,7 +399,7 @@ public void repeatUntilValue_throwsIfValueIsNeverTrue(Execution execution) {
         .repeatForDuration(Duration.ofSeconds(2));
 
     assertThatThrownBy(() -> execution.execute(concurrencyRule))
-        .isInstanceOf(ComparisonFailure.class);
+        .isInstanceOf(AssertionFailedError.class);
     assertThat(invoked.get()).isTrue();
   }
 
@@ -519,8 +519,8 @@ public void runManyThreads(Execution execution) {
     assertThat(errors.get(0)).isInstanceOf(AssertionError.class)
         .hasMessageContaining(IOException.class.getName());
     assertThat(errors.get(1)).isInstanceOf(AssertionError.class)
-        .hasMessageContaining("[successful] value")
-        .hasMessageContaining("[wrong] value");
+        .hasMessageContaining("successful value")
+        .hasMessageContaining("wrong value");
     assertThat(errors.get(2)).hasMessageContaining("foo")
         .isInstanceOf(IOException.class);
   }