Copilot commented on code in PR #19878:
URL: https://github.com/apache/druid/pull/19878#discussion_r3717412216


##########
extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/cache/JdbcExtractionNamespaceTest.java:
##########
@@ -85,8 +90,51 @@ public class JdbcExtractionNamespaceTest
       "empty string", new String[]{"empty string", "0"}
   );
 
+  private static class DerbyConnectorExtension implements BeforeEachCallback, 
AfterEachCallback
+  {
+    private final TestDerbyConnector connector = new TestDerbyConnector();
+
+    @Override
+    public void beforeEach(ExtensionContext context)
+    {
+      connector.createDatabase();
+    }
+
+    @Override
+    public void afterEach(ExtensionContext context)
+    {
+      try {
+        new DBI(connector.getJdbcUri() + ";drop=true").open().close();
+      }
+      catch (UnableToObtainConnectionException e) {
+        final SQLException cause = (SQLException) e.getCause();
+        Assertions.assertEquals(
+            "08006",
+            cause.getSQLState(),
+            StringUtils.format("Derby not shutdown: [%s]", cause)
+        );
+      }

Review Comment:
   `UnableToObtainConnectionException#getCause()` is cast to `SQLException` 
without checking for null/type, which can turn an expected Derby shutdown path 
into a `ClassCastException`/NPE and mask the real failure. Consider asserting 
the cause type before casting (similar to 
`TestDerbyConnector.DerbyConnectorRule`).



##########
extensions-core/lookups-cached-global/src/test/java/org/apache/druid/server/lookup/namespace/JdbcCacheGeneratorTest.java:
##########
@@ -83,32 +81,32 @@ public void setup()
   @Test
   public void indicatesMissingJdbcJarsWithTsColumn()
   {
-    String tsColumn = "tsColumn";
-    JdbcExtractionNamespace missingJarNamespace = 
createJdbcExtractionNamespace(
-        MISSING_METADATA_STORAGE_CONNECTOR_CONFIG,
-        tsColumn
-    );
-
-    exception.expect(IllegalStateException.class);
-    exception.expectMessage(MISSING_JDB_DRIVER_JAR_MSG);
-
-    target.generateCache(missingJarNamespace, KEY, LAST_VERSION, 
CACHE_MANAGER.allocateCache());
+    Throwable exception = assertThrows(IllegalStateException.class, () -> {
+      String tsColumn = "tsColumn";
+      JdbcExtractionNamespace missingJarNamespace = 
createJdbcExtractionNamespace(
+          MISSING_METADATA_STORAGE_CONNECTOR_CONFIG,
+          tsColumn
+      );

Review Comment:
   Code style: AGENTS.md requires using `final` for locals that are not 
reassigned. These new locals can be `final` to match the project convention.
   
   This issue also appears on line 99 of the same file.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to