Migrates all tests and fixtures from JUnit 4.x to Jupiter

Signed-off-by: Adrian Cole <adrian@tetrate.io>

RELEASE.md

@@ -60,12 +60,12 @@ Note: the command below is more complex than a normal project because this
 project has a bom.
 ```bash
 $ mvn=$PWD/mvnw
-$ for p in ./bom .; do
+$ for p in ./brave-bom .; do
  (cd $p
  $mvn versions:set -DnewVersion=2.17.1-SNAPSHOT -DgenerateBackupPoms=false
  $mvn -o clean install -DskipTests
  $mvn com.mycila:license-maven-plugin:format
- $mvn versions:set -DnewVersion=2.17.0-SNAPSHOT -DgenerateBackupPoms=false)
+ $mvn versions:set -DnewVersion=5.17.0-SNAPSHOT -DgenerateBackupPoms=false)
  done
 $ git commit -asm"Adjusts copyright headers for this year"
 ```

brave-bom/pom.xml

@@ -18,7 +18,7 @@
 
   <groupId>io.zipkin.brave</groupId>
   <artifactId>brave-bom</artifactId>
-  <version>5.16.1-SNAPSHOT</version>
+  <version>5.17.0-SNAPSHOT</version>
   <name>Brave BOM</name>
   <description>Bill Of Materials POM for all Brave artifacts</description>
   <packaging>pom</packaging>

brave-tests/pom.xml

@@ -18,7 +18,7 @@
   <parent>
     <groupId>io.zipkin.brave</groupId>
     <artifactId>brave-parent</artifactId>
-    <version>5.16.1-SNAPSHOT</version>
+    <version>5.17.0-SNAPSHOT</version>
   </parent>
   <modelVersion>4.0.0</modelVersion>
 
@@ -36,9 +36,10 @@
       <version>${project.version}</version>
     </dependency>
     <dependency>
-      <groupId>junit</groupId>
-      <artifactId>junit</artifactId>
-      <version>${junit.version}</version>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-api</artifactId>
+      <version>${junit-jupiter.version}</version>
+      <scope>provided</scope>
     </dependency>
     <dependency>
       <groupId>org.assertj</groupId>

brave-tests/src/main/java/brave/test/ITRemote.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2020 The OpenZipkin Authors
+ * Copyright 2013-2023 The OpenZipkin Authors
  *
  * 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
@@ -30,13 +30,15 @@
 import brave.sampler.Sampler;
 import java.io.Closeable;
 import java.io.IOException;
+import java.lang.reflect.Method;
 import java.util.Collections;
-import org.junit.After;
-import org.junit.Rule;
-import org.junit.rules.DisableOnDebug;
-import org.junit.rules.TestName;
-import org.junit.rules.TestRule;
-import org.junit.rules.Timeout;
+import java.util.Optional;
+import java.util.concurrent.TimeUnit;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.TestInfo;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.extension.RegisterExtension;
 
 import static brave.internal.InternalPropagation.FLAG_LOCAL_ROOT;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -50,7 +52,18 @@
  *   <li>{@link StrictCurrentTraceContext} double-checks threads don't leak contexts</li>
  *   <li>{@link IntegrationTestSpanHandler} helps avoid race conditions or accidental errors</li>
  * </ul></pre>
+ *
+ * <h2>Timeout</h2>
+ * We use a global rule instead of surefire config as this could be executed in gradle, sbt, etc.
+ * This way, there's visibility on which method hung without asking the end users to edit build
+ * config.
+ *
+ * <p>Normal tests will pass in less than 5 seconds. This timeout is set to 20 to be higher than
+ * needed even in an overloaded CI server or extreme garbage collection pause.
  */
+@Timeout(value = 20, unit = TimeUnit.MINUTES)
+// ^-- TODO: see if we can disable this on debug, though 20m debug time should be ok.
+// ^-- TODO: verify if this value is per method.
 public abstract class ITRemote {
   static {
     SamplingFlags.NOT_SAMPLED.toString(); // ensure InternalPropagation is wired for tests
@@ -72,23 +85,16 @@ public abstract class ITRemote {
    */
   public static final String BAGGAGE_FIELD_KEY = "user_id";
 
-  /**
-   * We use a global rule instead of surefire config as this could be executed in gradle, sbt, etc.
-   * This way, there's visibility on which method hung without asking the end users to edit build
-   * config.
-   *
-   * <p>Normal tests will pass in less than 5 seconds. This timeout is set to 20 to be higher than
-   * needed even in a an overloaded CI server or extreme garbage collection pause.
-   */
-  @Rule public TestRule globalTimeout = new DisableOnDebug(Timeout.seconds(20)); // max per method
-  @Rule public IntegrationTestSpanHandler testSpanHandler = new IntegrationTestSpanHandler();
-  @Rule public TestName testName = new TestName();
+  @RegisterExtension
+  protected IntegrationTestSpanHandler testSpanHandler = new IntegrationTestSpanHandler();
+
+  protected String testName;
 
   /** Returns a trace context for use in propagation tests. */
   protected TraceContext newTraceContext(SamplingFlags flags) {
     long id = System.nanoTime(); // Random enough as tests are run serially anyway
 
-    // Simulate a new local root root, but without the dependency on Tracer to create it.
+    // Simulate a new local root, but without the dependency on Tracer to create it.
     TraceContext context = InternalPropagation.instance.newTraceContext(
       InternalPropagation.instance.flags(flags) | FLAG_LOCAL_ROOT,
       0L,
@@ -158,7 +164,7 @@ protected Tracing.Builder tracingBuilder(Sampler sampler) {
    * }
    * }</pre>
    */
-  @After public void close() throws Exception {
+  @AfterEach protected void close() throws Exception {
     Tracing current = Tracing.current();
     if (current != null) current.close();
     checkForLeakedScopes();
@@ -239,4 +245,11 @@ protected void assertNoError(MutableSpan result) {
   protected void assertNoErrorTag(MutableSpan result) {
     IntegrationTestSpanHandler.assertNoErrorTag(result);
   }
+
+  @BeforeEach void setupTestName(TestInfo testInfo) {
+    Optional<Method> testMethod = testInfo.getTestMethod();
+    if (testMethod.isPresent()) {
+      this.testName = testMethod.get().getName();
+    }
+  }
 }

brave-tests/src/main/java/brave/test/IntegrationTestSpanHandler.java

@@ -20,33 +20,36 @@
 import brave.internal.Platform;
 import brave.internal.handler.OrphanTracker;
 import brave.propagation.TraceContext;
+import java.lang.reflect.Method;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.regex.Pattern;
-import org.junit.After;
-import org.junit.AssumptionViolatedException;
-import org.junit.rules.TestRule;
-import org.junit.runner.Description;
-import org.junit.runners.model.Statement;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.extension.ExtensionContext;
+import org.junit.jupiter.api.extension.InvocationInterceptor;
+import org.junit.jupiter.api.extension.ReflectiveInvocationContext;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+;
+
 /**
  * This is a special span reporter for remote integration tests.
  *
  * <p>Ex. The following is similar to our base test class {@link ITRemote}:
  * <pre>{@code
- * @Rule public IntegrationTestSpanHandler testSpanHandler = new IntegrationTestSpanHandler();
+ * @RegisterExtension IntegrationTestSpanHandler testSpanHandler = new IntegrationTestSpanHandler();
  *
  * Tracing tracing = Tracing.newBuilder().addSpanHandler(testSpanHandler).build();
  *
- * @After public void close() {
+ * @AfterEach void close() {
  *   tracing.close();
  * }
  *
- * @Test public void onTransportException_setError() {
+ * @Test void onTransportException_setError() {
  *   server.stop();
  *
  *   assertThatThrownBy(() -> client.get().sayHello("jorge"))
@@ -82,7 +85,7 @@
  * finished. An exception or bug could cause this (for example, the error handling route not calling
  * {@link brave.Span#finish()}).
  *
- * <p>If a test fails on {@link After}, it can mean that your test created a span, but didn't
+ * <p>If a test fails on {@link AfterEach}, it can mean that your test created a span, but didn't
  * {@link BlockingQueue#take()} it off the queue. If you are testing something that creates a span,
  * you may not want to verify each one. In this case, at least take them similar to below:
  *
@@ -103,18 +106,18 @@
  * If something written to work on one thread is suddenly working on two threads, tests can fail
  * "randomly", perhaps not until an unrelated change to JRE. When tests fail, they also make it
  * impossible to release new code until we disable the test or fix it. Bugs or race conditions
- * instrumentation can be very time consuming to solve. For example, they can appear as "flakes" in
+ * instrumentation can be very time-consuming to solve. For example, they can appear as "flakes" in
  * CI servers such as Travis, which can be near impossible to debug.
  *
  * <p>Bottom-line is that we accept that strict tests are harder up front, and not necessary for a
  * few types of blocking client instrumentation. However, the majority of remote instrumentation
- * have to concern themselves with multi-threaded behavior and if we always do, the chances of
+ * have to concern themselves with multithreaded behavior and if we always do, the chances of
  * builds breaking are less.
  *
  * @see TestSpanHandler
  * @since 5.12
  */
-public final class IntegrationTestSpanHandler extends SpanHandler implements TestRule {
+public final class IntegrationTestSpanHandler extends SpanHandler implements InvocationInterceptor {
   static final String ANY_STRING = ".+";
 
   /**
@@ -142,7 +145,7 @@ public IntegrationTestSpanHandler() {
   }
 
   /**
-   * Call this before throwing an {@link AssumptionViolatedException}, when there's a chance a span
+   * Call this before using an {@link Assumptions#assumeTrue(boolean)}, when there's a chance a span
    * was finished.
    *
    * <p>This was made for detecting features in HTTP server testing via 404. When 404 itself is
@@ -386,17 +389,16 @@ public boolean begin(TraceContext context, MutableSpan span, @Nullable TraceCont
     return true;
   }
 
-  @Override public Statement apply(Statement base, Description description) {
-    return new Statement() {
-      @Override public void evaluate() throws Throwable {
-        try {
-          base.evaluate();
-          assertSpansConsumed();
-        } finally {
-          spans.clear();
-        }
-      }
-    };
+  @Override
+  public void interceptTestMethod(Invocation<Void> invocation,
+    ReflectiveInvocationContext<Method> invocationContext, ExtensionContext extensionContext)
+    throws Throwable {
+    try {
+      invocation.proceed();
+      assertSpansConsumed();
+    } finally {
+      spans.clear();
+    }
   }
 
   @Override public String toString() {

brave-tests/src/main/java/brave/test/TestSpanHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2020 The OpenZipkin Authors
+ * Copyright 2013-2023 The OpenZipkin Authors
  *
  * 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
@@ -20,7 +20,7 @@
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
-import org.junit.rules.TestRule;
+import org.junit.jupiter.api.extension.Extension;
 
 /**
  * Simpler variant of {@link IntegrationTestSpanHandler} appropriate for single-threaded
@@ -31,11 +31,11 @@
  * TestSpanHandler spans = new TestSpanHandler();
  * Tracing tracing = Tracing.newBuilder().addSpanHandler(spans).build();
  *
- * @After public void close() {
+ * @AfterEach void close() {
  *   tracing.close();
  * }
  *
- * @Test public void test() {
+ * @Test void test() {
  *   tracing.tracer().startScopedSpan("foo").finish();
  *
  *   assertThat(spans.get(0).name())
@@ -44,7 +44,7 @@
  * }</pre>
  *
  * <h3>Comparison with {@link IntegrationTestSpanHandler}</h3>
- * It is possible to use this type in multi-threaded tests, but there are usually problems that
+ * It is possible to use this type in multithreaded tests, but there are usually problems that
  * arise better solved by {@link IntegrationTestSpanHandler}. Here are a few examples.
  *
  * <p>Multi-threaded tests typically end up with timing issues which can lead to broken builds (aka
@@ -59,7 +59,7 @@
  * utilities made for remote spans, such as {@link IntegrationTestSpanHandler#takeRemoteSpan(Span.Kind)}.
  *
  * <p>It is a common instrumentation bug to accidentally create redundant spans. {@link
- * IntegrationTestSpanHandler} is a {@link TestRule}, which verifies all spans are accounted for.
+ * IntegrationTestSpanHandler} is an {@link Extension}, which verifies all spans are accounted for.
  *
  * @see IntegrationTestSpanHandler
  * @since 5.12

brave-tests/src/main/java/brave/test/propagation/B3SingleFormatClassLoaderTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2020 The OpenZipkin Authors
+ * Copyright 2013-2023 The OpenZipkin Authors
  *
  * 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
@@ -14,12 +14,12 @@
 package brave.test.propagation;
 
 import brave.propagation.B3SingleFormat;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 import static brave.test.util.ClassLoaders.assertRunIsUnloadable;
 
-public class B3SingleFormatClassLoaderTest {
-  @Test public void unloadable_afterBasicUsage() {
+class B3SingleFormatClassLoaderTest {
+  @Test void unloadable_afterBasicUsage() {
     assertRunIsUnloadable(BasicUsage.class, getClass().getClassLoader());
   }