dan-s1 commented on code in PR #10457:
URL: https://github.com/apache/nifi/pull/10457#discussion_r2456138341


##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestListDatabaseTables.java:
##########
@@ -122,25 +100,16 @@ public void testListTablesNoCount() throws Exception {
     }
 
     @Test
-    public void testListTablesWithCount() throws Exception {
+    void testListTablesWithCount() throws Exception {
         runner.setProperty(ListDatabaseTables.INCLUDE_COUNT, "true");
 
-        // load test data to database
-        final Connection con = ((DBCPService) 
runner.getControllerService("dbcp")).getConnection();
-        Statement stmt = con.createStatement();
-
-        try {
-            stmt.execute("drop table TEST_TABLE1");
-            stmt.execute("drop table TEST_TABLE2");
-        } catch (final SQLException ignored) {
-            // Do nothing, may not have existed
+        try (Statement stmt = createStatement()) {
+            stmt.execute("create table TEST_TABLE1 (id integer not null, val1 
integer, val2 integer, constraint my_pk1 primary key (id))");
+            stmt.execute("insert into TEST_TABLE1 (id, val1, val2) VALUES (0, 
NULL, 1)");
+            stmt.execute("insert into TEST_TABLE1 (id, val1, val2) VALUES (1, 
1, 1)");

Review Comment:
   These inserts are also hard coded numerous times and would benefit to be in 
static final variable(s).



##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestListDatabaseTables.java:
##########
@@ -16,102 +16,80 @@
  */
 package org.apache.nifi.processors.standard;
 
-import org.apache.nifi.controller.AbstractControllerService;
-import org.apache.nifi.dbcp.DBCPService;
-import org.apache.nifi.processor.exception.ProcessException;
 import org.apache.nifi.serialization.record.MockRecordWriter;
 import org.apache.nifi.util.MockFlowFile;
 import org.apache.nifi.util.TestRunner;
 import org.apache.nifi.util.TestRunners;
-import org.apache.nifi.util.file.FileUtils;
 import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.condition.DisabledOnOs;
-import org.junit.jupiter.api.condition.OS;
+import org.junit.jupiter.api.io.TempDir;
 
-import java.io.File;
-import java.io.IOException;
+import java.nio.file.Path;
 import java.sql.Connection;
-import java.sql.DriverManager;
 import java.sql.SQLException;
-import java.sql.SQLNonTransientConnectionException;
 import java.sql.Statement;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
-/**
- * Unit tests for ListDatabaseTables processor.
- */
-public class TestListDatabaseTables {
+class TestListDatabaseTables {
+
+    private static final String SERVICE_ID = 
EmbeddedDatabaseConnectionService.class.getSimpleName();
+
+    private static EmbeddedDatabaseConnectionService service;
 
     TestRunner runner;
-    ListDatabaseTables processor;
 
-    private final static String DB_LOCATION = "target/db_ldt";
+    ListDatabaseTables processor;
 
     @BeforeAll
-    public static void setupBeforeClass() {
-        System.setProperty("derby.stream.error.file", "target/derby.log");
-
-        // remove previous test database, if any
-        final File dbLocation = new File(DB_LOCATION);
-        try {
-            FileUtils.deleteFile(dbLocation, true);
-        } catch (IOException ignored) {
-            // Do nothing, may not have existed
-        }
+    static void setService(@TempDir final Path databaseLocation) {
+        service = new EmbeddedDatabaseConnectionService(databaseLocation);
     }
 
     @AfterAll
-    public static void cleanUpAfterClass() throws Exception {
-        try {
-            DriverManager.getConnection("jdbc:derby:" + DB_LOCATION + 
";shutdown=true");
-        } catch (SQLNonTransientConnectionException ignored) {
-            // Do nothing, this is what happens at Derby shutdown
-        }
-        // remove previous test database, if any
-        final File dbLocation = new File(DB_LOCATION);
-        try {
-            FileUtils.deleteFile(dbLocation, true);
-        } catch (IOException ignored) {
-            // Do nothing, may not have existed
-        }
-        System.clearProperty("derby.stream.error.file");
+    static void shutdown() {
+        service.close();
     }
 
     @BeforeEach
-    public void setUp() throws Exception {
+    void setRunner() throws Exception {
         processor = new ListDatabaseTables();
-        final DBCPService dbcp = new DBCPServiceSimpleImpl();
-        final Map<String, String> dbcpProperties = new HashMap<>();
 
         runner = TestRunners.newTestRunner(ListDatabaseTables.class);
-        runner.addControllerService("dbcp", dbcp, dbcpProperties);
-        runner.enableControllerService(dbcp);
-        runner.setProperty(ListDatabaseTables.DBCP_SERVICE, "dbcp");
+        runner.addControllerService(SERVICE_ID, service);
+        runner.enableControllerService(service);
+        runner.setProperty(ListDatabaseTables.DBCP_SERVICE, SERVICE_ID);
     }
 
-    @Test
-    public void testListTablesNoCount() throws Exception {
-
-        // load test data to database
-        final Connection con = ((DBCPService) 
runner.getControllerService("dbcp")).getConnection();
-        Statement stmt = con.createStatement();
+    @AfterEach
+    void dropTables() {
+        final List<String> tables = List.of(
+                "TEST_TABLE1",
+                "TEST_TABLE2"
+        );
+
+        for (final String table : tables) {
+            try (
+                    Connection connection = service.getConnection();
+                    Statement statement = connection.createStatement()
+            ) {
+                statement.execute("DROP TABLE %s".formatted(table));
+            } catch (final SQLException ignored) {
 
-        try {
-            stmt.execute("drop table TEST_TABLE1");
-            stmt.execute("drop table TEST_TABLE2");
-        } catch (final SQLException ignored) {
-            // Do nothing, may not have existed
+            }
         }
+    }
 
-        stmt.execute("create table TEST_TABLE1 (id integer not null, val1 
integer, val2 integer, constraint my_pk1 primary key (id))");
-        stmt.execute("create table TEST_TABLE2 (id integer not null, val1 
integer, val2 integer, constraint my_pk2 primary key (id))");
+    @Test
+    void testListTablesNoCount() throws Exception {
+        try (Statement stmt = createStatement()) {
+            stmt.execute("create table TEST_TABLE1 (id integer not null, val1 
integer, val2 integer, constraint my_pk1 primary key (id))");
+            stmt.execute("create table TEST_TABLE2 (id integer not null, val1 
integer, val2 integer, constraint my_pk2 primary key (id))");

Review Comment:
   These two exact statement to create the tables are hard coded five times. It 
would seem this would benefit to have static final variables.



-- 
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]

Reply via email to