Move per-class fixture state from static to instance fields in lettuce-5.1 abstract test bases

When an abstract test base is shared across multiple concrete subclasses run in the same JVM fork, static fixture state (containers, clients, host/port, the AutoCleanupExtension itself, etc.) becomes JVM-wide state that subclasses fight over.

Two concrete failure modes were observed/possible:

1. AutoCleanupExtension.afterAll latches rootContext on the first subclass's @BeforeAll and never resets it, so deferAfterAll callbacks only run for the first subclass; subsequent subclasses leak their containers/clients/connections.

2. The auth subclass's redisServer.withCommand("--requirepass password") becomes a no-op when an earlier subclass has already started the (shared static) container — the suspected root cause of the LettuceSyncClientAuthTest flake.

Fix: convert per-class state to instance fields on classes already annotated @testinstance(PER_CLASS), so each concrete subclass gets its own container/client lifecycle. spanName(String) becomes an instance method since it reads instance host/port. The auth subclass no longer needs a 'set back' withCommand cleanup, since each class has its own container instance.

Codifies the pattern in .github/agents/knowledge/testing-general-patterns.md.

Author: trask

.github/agents/knowledge/README.md

@@ -18,7 +18,7 @@ Load only files relevant to the current scope to reduce noise and avoid over-con
 | `javaagent-singletons-patterns.md` | `*Singletons`, `*SpanNaming`, and similar holder classes; singleton accessors; callers of singleton accessors/fields |
 | `library-patterns.md` | Library instrumentation telemetry, builder, getter, or setter pattern changes |
 | `module-naming.md` | New or renamed modules or packages; settings includes |
-| `testing-general-patterns.md` | Test files in scope — assertion style, test method signatures and throws clauses, resource cleanup patterns, attribute assertion patterns, `satisfies()` lambda usage |
+| `testing-general-patterns.md` | Test files in scope — assertion style, test method signatures and throws clauses, resource cleanup patterns, abstract test base class state shape, attribute assertion patterns, `satisfies()` lambda usage |
 | `testing-experimental-flags.md` | `testExperimental` task or experimental span-attribute assertions |
 | `testing-semconv-stability.md` | Semconv opt-in modes, `emitOld*`/`emitStable*`, `maybeStable`, Semconv test tasks |
 

.github/agents/knowledge/testing-general-patterns.md

@@ -77,6 +77,34 @@
 - If the test intentionally closes the resource mid-test or asserts behavior around explicit
   close, keep the direct close or try-with-resources in the test body.
 
+## Abstract Test Base Classes — Per-Class State Goes On Instance Fields
+
+When an abstract test base is shared across multiple concrete subclasses run in the same
+JVM fork, treat per-class fixture state (containers, clients, connections, host/port, the
+`AutoCleanupExtension` itself, etc.) as **instance state on a `@TestInstance(PER_CLASS)`
+class**, not as `static` fields.
+
+- Annotate the base with `@TestInstance(TestInstance.Lifecycle.PER_CLASS)`. JUnit then creates
+  one instance per concrete subclass and reuses it for every `@Test` in that class, so
+  instance fields give the same once-per-class lifecycle that `static` fields used to provide,
+  without leaking across subclasses.
+- Make `redisServer` / `kafkaContainer` / `client` / `host` / `port` / etc. instance fields
+  (`protected`, not `protected static`). Each concrete subclass then gets its own container
+  and client, freshly configured by its own `@BeforeAll`.
+- Make `@RegisterExtension AutoCleanupExtension cleanup` an **instance** field, not `static`.
+  A shared `static` `AutoCleanupExtension` only runs `deferAfterAll(...)` callbacks for the
+  first subclass JUnit invokes — its `rootContext` is set on first `beforeAll` and never
+  reset — so subsequent subclasses leak deferred resources (containers, clients, connections).
+- Helpers that read instance fields (e.g. `spanName(String)` reading `host`/`port`) must
+  also be instance methods, not `static`.
+- This is also the right shape when a subclass needs to mutate the container's builder
+  configuration (e.g. `withCommand("--requirepass password")`). With shared `static` state the
+  earlier subclass may have already started the container, so `withCommand` on the running
+  instance has no effect; with per-instance state each subclass mutates and starts its own
+  fresh container.
+- Reserve `static` for true JVM-wide constants (`DB_INDEX`, loggers, `*_HASH_MAP` literals,
+  pure helpers like `addExtraAttributes(...)` / `assertCommandEncodeEvents(...)`).
+
 ## Span Attribute Assertions
 
 - Use `span.hasAttributesSatisfyingExactly(...)` with `equalTo(...)`/`satisfies(...)` for

instrumentation/lettuce/lettuce-5.1/javaagent/src/testCompatibility/java/io/opentelemetry/javaagent/instrumentation/lettuce/v5_1/LettuceCompatibilityTest.java

@@ -34,7 +34,7 @@ protected RedisClient createClient(String uri) {
     return RedisClient.create(uri);
   }
 
-  private static RedisCommands<String, String> syncCommands;
+  private RedisCommands<String, String> syncCommands;
 
   @BeforeAll
   void setUp() throws UnknownHostException {
@@ -54,7 +54,7 @@ void setUp() throws UnknownHostException {
   }
 
   @AfterAll
-  static void cleanUp() {
+  void cleanUp() {
     connection.close();
     redisClient.shutdown();
     redisServer.stop();

instrumentation/lettuce/lettuce-5.1/testing/src/main/java/io/opentelemetry/instrumentation/lettuce/v5_1/AbstractLettuceAsyncClientTest.java

@@ -51,16 +51,16 @@
 
 @SuppressWarnings({"InterruptedExceptionSwallowed", "deprecation"}) // using deprecated semconv
 public abstract class AbstractLettuceAsyncClientTest extends AbstractLettuceClientTest {
-  private static String dbUriNonExistent;
-  private static int incorrectPort;
+  private String dbUriNonExistent;
+  private int incorrectPort;
 
   private static final ImmutableMap<String, String> TEST_HASH_MAP =
       ImmutableMap.of(
           "firstname", "John",
           "lastname", "Doe",
           "age", "53");
 
-  private static RedisAsyncCommands<String, String> asyncCommands;
+  private RedisAsyncCommands<String, String> asyncCommands;
 
   @BeforeAll
   void setUp() throws UnknownHostException {

instrumentation/lettuce/lettuce-5.1/testing/src/main/java/io/opentelemetry/instrumentation/lettuce/v5_1/AbstractLettuceClientTest.java

@@ -28,22 +28,22 @@
 public abstract class AbstractLettuceClientTest {
   private static final Logger logger = LoggerFactory.getLogger(AbstractLettuceClientTest.class);
 
-  @RegisterExtension static final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
+  @RegisterExtension final AutoCleanupExtension cleanup = AutoCleanupExtension.create();
 
   protected static final int DB_INDEX = 0;
 
-  protected static GenericContainer<?> redisServer =
+  protected GenericContainer<?> redisServer =
       new GenericContainer<>("redis:6.2.3-alpine")
           .withExposedPorts(6379)
           .withLogConsumer(new Slf4jLogConsumer(logger))
           .waitingFor(Wait.forLogMessage(".*Ready to accept connections.*", 1));
 
-  protected static RedisClient redisClient;
-  protected static StatefulRedisConnection<String, String> connection;
-  protected static String host;
-  protected static String ip;
-  protected static int port;
-  protected static String embeddedDbUri;
+  protected RedisClient redisClient;
+  protected StatefulRedisConnection<String, String> connection;
+  protected String host;
+  protected String ip;
+  protected int port;
+  protected String embeddedDbUri;
 
   protected abstract RedisClient createClient(String uri);
 
@@ -91,7 +91,7 @@ protected static void assertCommandEncodeEvents(SpanData span) {
             event -> event.hasName("redis.encode.end"));
   }
 
-  protected static String spanName(String operation) {
+  protected String spanName(String operation) {
     return spanName(operation, host, port);
   }
 

instrumentation/lettuce/lettuce-5.1/testing/src/main/java/io/opentelemetry/instrumentation/lettuce/v5_1/AbstractLettuceReactiveClientTest.java

@@ -35,7 +35,7 @@
 @SuppressWarnings({"InterruptedExceptionSwallowed", "deprecation"}) // using deprecated semconv
 public abstract class AbstractLettuceReactiveClientTest extends AbstractLettuceClientTest {
 
-  protected static RedisReactiveCommands<String, String> reactiveCommands;
+  protected RedisReactiveCommands<String, String> reactiveCommands;
 
   @BeforeAll
   void setUp() throws UnknownHostException {

instrumentation/lettuce/lettuce-5.1/testing/src/main/java/io/opentelemetry/instrumentation/lettuce/v5_1/AbstractLettuceSyncClientAuthTest.java

@@ -40,9 +40,6 @@ public abstract class AbstractLettuceSyncClientAuthTest extends AbstractLettuceC
   void setUp() throws UnknownHostException {
     redisServer = redisServer.withCommand("redis-server", "--requirepass password");
     redisServer.start();
-    // Set back so other tests don't fail due to NOAUTH error.
-    cleanup.deferAfterAll(
-        () -> redisServer = redisServer.withCommand("redis-server", "--requirepass \"\""));
     cleanup.deferAfterAll(redisServer::stop);
 
     host = redisServer.getHost();

instrumentation/lettuce/lettuce-5.1/testing/src/main/java/io/opentelemetry/instrumentation/lettuce/v5_1/AbstractLettuceSyncClientTest.java

@@ -53,15 +53,15 @@
 @SuppressWarnings("deprecation") // using deprecated semconv
 public abstract class AbstractLettuceSyncClientTest extends AbstractLettuceClientTest {
 
-  private static String dbUriNonExistent;
+  private String dbUriNonExistent;
 
   private static final ImmutableMap<String, String> TEST_HASH_MAP =
       ImmutableMap.of(
           "firstname", "John",
           "lastname", "Doe",
           "age", "53");
 
-  private static RedisCommands<String, String> syncCommands;
+  private RedisCommands<String, String> syncCommands;
 
   @BeforeAll
   void setUp() throws UnknownHostException {