exceptionfactory commented on code in PR #11761:
URL: https://github.com/apache/nifi/pull/11761#discussion_r4275878497
##########
nifi-extension-bundles/nifi-extension-utils/nifi-database-utils/src/test/java/org/apache/nifi/util/db/AbstractConnectionTest.java:
##########
@@ -37,7 +39,18 @@ static void setConnectionUrl(@TempDir final Path tempDir) {
} catch (final ClassNotFoundException e) {
throw new IllegalStateException("Driver Class [%s] not
found".formatted(DRIVER_CLASS), e);
}
- connectionUrl = CONNECTION_URL_FORMAT.formatted(tempDir);
+ connectionUrl =
CONNECTION_URL_FORMAT.formatted(tempDir.toAbsolutePath());
+ }
+
+ @AfterAll
+ public static void shutdownDatabase() throws SQLException {
+ if (connectionUrl != null) {
Review Comment:
I don't think the null check is necessary, since every test would fail if it
is null
##########
nifi-extension-bundles/nifi-extension-utils/nifi-database-utils/src/test/java/org/apache/nifi/util/db/AbstractConnectionTest.java:
##########
@@ -37,7 +39,18 @@ static void setConnectionUrl(@TempDir final Path tempDir) {
} catch (final ClassNotFoundException e) {
throw new IllegalStateException("Driver Class [%s] not
found".formatted(DRIVER_CLASS), e);
}
- connectionUrl = CONNECTION_URL_FORMAT.formatted(tempDir);
+ connectionUrl =
CONNECTION_URL_FORMAT.formatted(tempDir.toAbsolutePath());
+ }
+
+ @AfterAll
+ public static void shutdownDatabase() throws SQLException {
+ if (connectionUrl != null) {
+ try (Connection conn = DriverManager.getConnection(connectionUrl,
"SA", "");
+ Statement stmt = conn.createStatement()) {
+ // Forces the engine to close immediately, bypassing active
pool connection state leaks
Review Comment:
I think it would be helpful to adjust the comment to state something like
"Engine shutdown closes all connections regardless of status"
##########
nifi-extension-bundles/nifi-extension-utils/nifi-database-utils/src/test/java/org/apache/nifi/util/db/AbstractConnectionTest.java:
##########
@@ -37,7 +39,18 @@ static void setConnectionUrl(@TempDir final Path tempDir) {
} catch (final ClassNotFoundException e) {
throw new IllegalStateException("Driver Class [%s] not
found".formatted(DRIVER_CLASS), e);
}
- connectionUrl = CONNECTION_URL_FORMAT.formatted(tempDir);
+ connectionUrl =
CONNECTION_URL_FORMAT.formatted(tempDir.toAbsolutePath());
+ }
+
+ @AfterAll
+ public static void shutdownDatabase() throws SQLException {
+ if (connectionUrl != null) {
+ try (Connection conn = DriverManager.getConnection(connectionUrl,
"SA", "");
Review Comment:
Are the username and password arguments required? It looks like they can be
removed.
##########
nifi-extension-bundles/nifi-standard-services/nifi-dbcp-service-bundle/nifi-dbcp-service/src/test/java/org/apache/nifi/dbcp/DBCPServiceTest.java:
##########
@@ -63,25 +66,55 @@ public class DBCPServiceTest {
private static final String NON_JDBC_URL =
"http://localhost:3306/database";;
private static final String DRIVER_CLASS = "org.hsqldb.jdbc.JDBCDriver";
- private static final String CONNECTION_URL_FORMAT = "jdbc:hsqldb:file:%s";
+ private static final String CONNECTION_URL_FORMAT =
"jdbc:hsqldb:file:%s/nifi_test_db";
private TestRunner runner;
private DBCPConnectionPool service;
+ private String connectionUrl;
+
+ private Path databasePath;
+
@BeforeEach
public void setService(@TempDir final Path tempDir) throws
InitializationException {
service = new DBCPConnectionPool();
runner = TestRunners.newTestRunner(NoOpProcessor.class);
runner.addControllerService(SERVICE_ID, service);
- final String url = CONNECTION_URL_FORMAT.formatted(tempDir);
- runner.setProperty(service, DBCPProperties.DATABASE_URL, url);
+ databasePath = tempDir.toAbsolutePath();
+ connectionUrl = CONNECTION_URL_FORMAT.formatted(databasePath);
+ runner.setProperty(service, DBCPProperties.DATABASE_URL,
connectionUrl);
runner.setProperty(service, DBCPProperties.DB_USER,
String.class.getSimpleName());
runner.setProperty(service, DBCPProperties.DB_PASSWORD,
String.class.getName());
runner.setProperty(service, DBCPProperties.DB_DRIVERNAME,
DRIVER_CLASS);
}
+ @AfterEach
+ public void shutdownDatabase() throws Exception {
+ runner.shutdown();
+
+ // Property validation tests may never create the database.
+ // Do not create it just to shut it down.
+ final Path propertiesFile =
databasePath.resolve("nifi_test_db.properties");
+ if (Files.exists(propertiesFile)) {
+ final Properties properties = new Properties();
+ properties.setProperty("user", String.class.getSimpleName());
+ properties.setProperty("password", String.class.getName());
Review Comment:
Are the user and password arguments required?
##########
nifi-extension-bundles/nifi-extension-utils/nifi-database-utils/src/test/java/org/apache/nifi/util/db/AbstractConnectionTest.java:
##########
@@ -16,17 +16,19 @@
*/
package org.apache.nifi.util.db;
+import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.io.TempDir;
import java.nio.file.Path;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;
+import java.sql.Statement;
abstract class AbstractConnectionTest {
private static final String DRIVER_CLASS = "org.hsqldb.jdbc.JDBCDriver";
- private static final String CONNECTION_URL_FORMAT = "jdbc:hsqldb:file:%s";
+ private static final String CONNECTION_URL_FORMAT =
"jdbc:hsqldb:file:%1$s/nifi_test_db";
Review Comment:
Perhaps just `nifi` is sufficient, instead of `nifi_test_db`
--
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]
