Re: [PR] [Feat][Connector-v2][JDBC][Oracle] Add Testcontainers-based unit tests [seatunnel]
zooo-code commented on code in PR #10435:
URL: https://github.com/apache/seatunnel/pull/10435#discussion_r2793669027
##
seatunnel-connectors-v2/connector-jdbc/src/test/java/org/apache/seatunnel/connectors/seatunnel/jdbc/internal/dialect/oracle/container/OracleDialectTest.java:
##
@@ -0,0 +1,218 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.oracle.container;
+
+import org.apache.seatunnel.api.table.catalog.TablePath;
+import
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.DatabaseIdentifier;
+import
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.oracle.OracleDialect;
+import org.apache.seatunnel.connectors.seatunnel.jdbc.source.JdbcSourceTable;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+/**
+ * Unit tests for OracleDialect using Testcontainers. Tests dialect-specific
functionality like
+ * quoting, SQL generation, and data sampling.
+ */
+@DisabledOnOs(OS.WINDOWS)
+public class OracleDialectTest extends AbstractOracleContainerTest {
Review Comment:
@chl-wxp Thanks for the review!
I've added all the requested tests as suggested.
(10 new tests)
1. DML Statements
- Covered Insert, Update, Delete, RowExists, and Upsert logic.
2. FieldNamedPreparedStatement
- Covered parameter parsing, duplicate handling, and execution.
All tests passed locally.
Thanks!
--
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]
Re: [PR] [Feat][Connector-v2][JDBC][Oracle] Add Testcontainers-based unit tests [seatunnel]
chl-wxp commented on code in PR #10435:
URL: https://github.com/apache/seatunnel/pull/10435#discussion_r2788107424
##
seatunnel-connectors-v2/connector-jdbc/src/test/java/org/apache/seatunnel/connectors/seatunnel/jdbc/internal/dialect/oracle/container/OracleDialectTest.java:
##
@@ -0,0 +1,218 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.oracle.container;
+
+import org.apache.seatunnel.api.table.catalog.TablePath;
+import
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.DatabaseIdentifier;
+import
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.oracle.OracleDialect;
+import org.apache.seatunnel.connectors.seatunnel.jdbc.source.JdbcSourceTable;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+/**
+ * Unit tests for OracleDialect using Testcontainers. Tests dialect-specific
functionality like
+ * quoting, SQL generation, and data sampling.
+ */
+@DisabledOnOs(OS.WINDOWS)
+public class OracleDialectTest extends AbstractOracleContainerTest {
Review Comment:
Add `getInsertIntoStatement()`, `getUpdateStatement()`,
`getDeleteStatement()`, `getRowExistsStatement()`, `getUpsertStatement()` and
other corresponding unit tests, and it is best to add
`FieldNamedPreparedStatement.prepareStatement()`.
--
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]
Re: [PR] [Feat][Connector-v2][JDBC][Oracle] Add Testcontainers-based unit tests [seatunnel]
zooo-code commented on PR #10435: URL: https://github.com/apache/seatunnel/pull/10435#issuecomment-3877198662 > Thank you for your contribution. Could you migrate the unit tests from the jdbc-e2e module to connector-jdbc later? Or could you submit them together this time Thanks for the approval! @LiJie20190102 I am open to both approaches. If you prefer merging them together in this PR, I can work on the migration now. Or, if you prefer keeping this PR focused, I can submit a separate PR for the migration later. Please let me know which one you prefer! -- 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]
Re: [PR] [Feat][Connector-v2][JDBC][Oracle] Add Testcontainers-based unit tests [seatunnel]
zooo-code commented on PR #10435: URL: https://github.com/apache/seatunnel/pull/10435#issuecomment-3852854499 I followed the pattern in #10252 PR, with a few adjustments to align with project conventions - Disabled tests on Windows due to missing Docker environment in CI - Renamed test classes to align with naming conventions (removed "Container") - Applied try-with-resources -- 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]
Re: [PR] [Feat][Connector-v2][JDBC][Oracle] Add Testcontainers-based unit tests [seatunnel]
zooo-code commented on code in PR #10435:
URL: https://github.com/apache/seatunnel/pull/10435#discussion_r2767904140
##
seatunnel-connectors-v2/connector-jdbc/src/test/java/org/apache/seatunnel/connectors/seatunnel/jdbc/internal/dialect/oracle/container/OracleDialectContainerTest.java:
##
@@ -0,0 +1,216 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.oracle.container;
+
+import org.apache.seatunnel.api.table.catalog.TablePath;
+import
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.DatabaseIdentifier;
+import
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.oracle.OracleDialect;
+import org.apache.seatunnel.connectors.seatunnel.jdbc.source.JdbcSourceTable;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+/**
+ * Unit tests for OracleDialect using Testcontainers. Tests dialect-specific
functionality like
+ * quoting, SQL generation, and data sampling.
+ */
+public class OracleDialectContainerTest extends AbstractOracleContainerTest {
+
+private static OracleDialect dialect;
+private static final String TEST_TABLE = "DIALECT_TEST_TABLE";
+
+@BeforeAll
+public static void setupDialect() throws SQLException {
+dialect = new OracleDialect();
+
+String createTableSql =
+String.format(
+"CREATE TABLE %s.%s ("
++ "ID NUMBER(10) PRIMARY KEY, "
++ "VARCHAR_COL VARCHAR2(100), "
++ "NUMBER_COL NUMBER(10,2), "
++ "DATE_COL DATE"
++ ")",
+quoteIdentifier(SCHEMA), quoteIdentifier(TEST_TABLE));
+
+try (Statement stmt = connection.createStatement()) {
+stmt.execute(createTableSql);
+}
+
+String insertSql =
+String.format(
+"INSERT INTO %s.%s (ID, VARCHAR_COL, NUMBER_COL,
DATE_COL) "
++ "VALUES (1, 'test1', 100.50, SYSDATE)",
+quoteIdentifier(SCHEMA), quoteIdentifier(TEST_TABLE));
+try (Statement stmt = connection.createStatement()) {
+stmt.execute(insertSql);
+// Note: Auto-commit is enabled by default, so no explicit commit
needed
+}
+}
+
+@Test
+public void testDialectName() {
+Assertions.assertEquals(DatabaseIdentifier.ORACLE,
dialect.dialectName());
+}
+
+@Test
+public void testQuoteIdentifier() {
+Assertions.assertEquals("\"table_name\"",
dialect.quoteIdentifier("table_name"));
+Assertions.assertEquals("\"COLUMN\"",
dialect.quoteIdentifier("COLUMN"));
+}
+
+@Test
+public void testTableIdentifierWithSchema() {
+// OracleDialect.tableIdentifier(String, String) ignores database
parameter
+String identifier = dialect.tableIdentifier(SCHEMA, TEST_TABLE);
+Assertions.assertTrue(identifier.contains(TEST_TABLE));
+}
+
+@Test
+public void testTableIdentifierWithTablePath() {
+TablePath tablePath = TablePath.of(null, SCHEMA, TEST_TABLE);
+String identifier = dialect.tableIdentifier(tablePath);
+Assertions.assertTrue(identifier.contains(SCHEMA));
+Assertions.assertTrue(identifier.contains(TEST_TABLE));
+}
+
+@Test
+public void testParse() {
+TablePath parsedPath = dialect.parse(SCHEMA + "." + TEST_TABLE);
+Assertions.assertEquals(SCHEMA, parsedPath.getSchemaName());
+Assertions.assertEquals(TEST_TABLE, parsedPath.getTableName());
+
+TablePath singlePath = dialect.parse(TEST_TABLE);
+Assertions.assertEquals(TEST_TABLE, singlePath.getTableName());
+}
+
+@Test
+public void testHashModForField() {
+String result = dialect.hashModForField("ID", 10);
+Assertions.assertTrue(result.contains("MOD"));
+Assertions.assertTrue(result.contains("ORA_HAS
Re: [PR] [Feat][Connector-v2][JDBC][Oracle] Add Testcontainers-based unit tests [seatunnel]
zooo-code commented on code in PR #10435:
URL: https://github.com/apache/seatunnel/pull/10435#discussion_r2767904140
##
seatunnel-connectors-v2/connector-jdbc/src/test/java/org/apache/seatunnel/connectors/seatunnel/jdbc/internal/dialect/oracle/container/OracleDialectContainerTest.java:
##
@@ -0,0 +1,216 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.oracle.container;
+
+import org.apache.seatunnel.api.table.catalog.TablePath;
+import
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.DatabaseIdentifier;
+import
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.oracle.OracleDialect;
+import org.apache.seatunnel.connectors.seatunnel.jdbc.source.JdbcSourceTable;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+import java.sql.Statement;
+
+/**
+ * Unit tests for OracleDialect using Testcontainers. Tests dialect-specific
functionality like
+ * quoting, SQL generation, and data sampling.
+ */
+public class OracleDialectContainerTest extends AbstractOracleContainerTest {
+
+private static OracleDialect dialect;
+private static final String TEST_TABLE = "DIALECT_TEST_TABLE";
+
+@BeforeAll
+public static void setupDialect() throws SQLException {
+dialect = new OracleDialect();
+
+String createTableSql =
+String.format(
+"CREATE TABLE %s.%s ("
++ "ID NUMBER(10) PRIMARY KEY, "
++ "VARCHAR_COL VARCHAR2(100), "
++ "NUMBER_COL NUMBER(10,2), "
++ "DATE_COL DATE"
++ ")",
+quoteIdentifier(SCHEMA), quoteIdentifier(TEST_TABLE));
+
+try (Statement stmt = connection.createStatement()) {
+stmt.execute(createTableSql);
+}
+
+String insertSql =
+String.format(
+"INSERT INTO %s.%s (ID, VARCHAR_COL, NUMBER_COL,
DATE_COL) "
++ "VALUES (1, 'test1', 100.50, SYSDATE)",
+quoteIdentifier(SCHEMA), quoteIdentifier(TEST_TABLE));
+try (Statement stmt = connection.createStatement()) {
+stmt.execute(insertSql);
+// Note: Auto-commit is enabled by default, so no explicit commit
needed
+}
+}
+
+@Test
+public void testDialectName() {
+Assertions.assertEquals(DatabaseIdentifier.ORACLE,
dialect.dialectName());
+}
+
+@Test
+public void testQuoteIdentifier() {
+Assertions.assertEquals("\"table_name\"",
dialect.quoteIdentifier("table_name"));
+Assertions.assertEquals("\"COLUMN\"",
dialect.quoteIdentifier("COLUMN"));
+}
+
+@Test
+public void testTableIdentifierWithSchema() {
+// OracleDialect.tableIdentifier(String, String) ignores database
parameter
+String identifier = dialect.tableIdentifier(SCHEMA, TEST_TABLE);
+Assertions.assertTrue(identifier.contains(TEST_TABLE));
+}
+
+@Test
+public void testTableIdentifierWithTablePath() {
+TablePath tablePath = TablePath.of(null, SCHEMA, TEST_TABLE);
+String identifier = dialect.tableIdentifier(tablePath);
+Assertions.assertTrue(identifier.contains(SCHEMA));
+Assertions.assertTrue(identifier.contains(TEST_TABLE));
+}
+
+@Test
+public void testParse() {
+TablePath parsedPath = dialect.parse(SCHEMA + "." + TEST_TABLE);
+Assertions.assertEquals(SCHEMA, parsedPath.getSchemaName());
+Assertions.assertEquals(TEST_TABLE, parsedPath.getTableName());
+
+TablePath singlePath = dialect.parse(TEST_TABLE);
+Assertions.assertEquals(TEST_TABLE, singlePath.getTableName());
+}
+
+@Test
+public void testHashModForField() {
+String result = dialect.hashModForField("ID", 10);
+Assertions.assertTrue(result.contains("MOD"));
+Assertions.assertTrue(result.contains("ORA_HAS
Re: [PR] [Feat][Connector-v2][JDBC][Oracle] Add Testcontainers-based unit tests [seatunnel]
zooo-code commented on code in PR #10435:
URL: https://github.com/apache/seatunnel/pull/10435#discussion_r2767899226
##
seatunnel-connectors-v2/connector-jdbc/src/test/java/org/apache/seatunnel/connectors/seatunnel/jdbc/internal/dialect/oracle/container/AbstractOracleContainerTest.java:
##
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.oracle.container;
+
+import
org.apache.seatunnel.connectors.seatunnel.jdbc.catalog.oracle.OracleCatalog;
+import
org.apache.seatunnel.connectors.seatunnel.jdbc.catalog.oracle.OracleURLParser;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.testcontainers.containers.OracleContainer;
+import org.testcontainers.utility.DockerImageName;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.time.Duration;
+
+/**
+ * Base class for Oracle Testcontainers-based unit tests. Provides shared
Oracle container setup and
+ * connection management.
+ */
+@DisabledOnOs(OS.WINDOWS)
+public abstract class AbstractOracleContainerTest {
+
+protected static final String ORACLE_IMAGE = "gvenzl/oracle-free:23-slim";
+protected static final String USERNAME = "TESTUSER";
+protected static final String PASSWORD = "testPassword";
+protected static final String DATABASE = "XE";
+protected static final String SCHEMA = USERNAME;
+
+protected static OracleContainer oracleContainer;
+protected static Connection connection;
+protected static OracleCatalog catalog;
+
+@BeforeAll
+public static void startContainer() throws SQLException {
+DockerImageName imageName =
+
DockerImageName.parse(ORACLE_IMAGE).asCompatibleSubstituteFor("gvenzl/oracle-xe");
+
+oracleContainer =
+new OracleContainer(imageName)
+.withDatabaseName(DATABASE)
+.withUsername(USERNAME)
+.withPassword(PASSWORD)
+.withSharedMemorySize(1073741824L) // 1GB shared memory
+.withStartupTimeout(Duration.ofMinutes(5)) // Increase
timeout for ARM
+.withEnv("ORACLE_CHARACTERSET", "AL32UTF8")
+.withReuse(false);
+
+oracleContainer.start();
+
+String jdbcUrl = oracleContainer.getJdbcUrl();
+connection =
+DriverManager.getConnection(
+jdbcUrl, oracleContainer.getUsername(),
oracleContainer.getPassword());
+
+catalog =
+new OracleCatalog(
+"oracle",
+oracleContainer.getUsername(),
+oracleContainer.getPassword(),
+OracleURLParser.parse(jdbcUrl),
+SCHEMA,
+null);
+catalog.open();
+}
+
+@AfterAll
+public static void stopContainer() throws SQLException {
+if (catalog != null) {
+catalog.close();
Review Comment:
Added try-catch blocks to ensure all resources are closed even if
catalog.close() throws an exception.
--
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]
Re: [PR] [Feat][Connector-v2][JDBC][Oracle] Add Testcontainers-based unit tests [seatunnel]
LiJie20190102 commented on code in PR #10435:
URL: https://github.com/apache/seatunnel/pull/10435#discussion_r2764261941
##
seatunnel-connectors-v2/connector-jdbc/src/test/java/org/apache/seatunnel/connectors/seatunnel/jdbc/internal/dialect/oracle/container/AbstractOracleContainerTest.java:
##
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package
org.apache.seatunnel.connectors.seatunnel.jdbc.internal.dialect.oracle.container;
+
+import
org.apache.seatunnel.connectors.seatunnel.jdbc.catalog.oracle.OracleCatalog;
+import
org.apache.seatunnel.connectors.seatunnel.jdbc.catalog.oracle.OracleURLParser;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.testcontainers.containers.OracleContainer;
+import org.testcontainers.utility.DockerImageName;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.time.Duration;
+
+/**
+ * Base class for Oracle Testcontainers-based unit tests. Provides shared
Oracle container setup and
+ * connection management.
+ */
+@DisabledOnOs(OS.WINDOWS)
+public abstract class AbstractOracleContainerTest {
+
+protected static final String ORACLE_IMAGE = "gvenzl/oracle-free:23-slim";
+protected static final String USERNAME = "TESTUSER";
+protected static final String PASSWORD = "testPassword";
+protected static final String DATABASE = "XE";
+protected static final String SCHEMA = USERNAME;
+
+protected static OracleContainer oracleContainer;
+protected static Connection connection;
+protected static OracleCatalog catalog;
+
+@BeforeAll
+public static void startContainer() throws SQLException {
+DockerImageName imageName =
+
DockerImageName.parse(ORACLE_IMAGE).asCompatibleSubstituteFor("gvenzl/oracle-xe");
+
+oracleContainer =
+new OracleContainer(imageName)
+.withDatabaseName(DATABASE)
+.withUsername(USERNAME)
+.withPassword(PASSWORD)
+.withSharedMemorySize(1073741824L) // 1GB shared memory
+.withStartupTimeout(Duration.ofMinutes(5)) // Increase
timeout for ARM
+.withEnv("ORACLE_CHARACTERSET", "AL32UTF8")
+.withReuse(false);
+
+oracleContainer.start();
+
+String jdbcUrl = oracleContainer.getJdbcUrl();
+connection =
+DriverManager.getConnection(
+jdbcUrl, oracleContainer.getUsername(),
oracleContainer.getPassword());
+
+catalog =
+new OracleCatalog(
+"oracle",
+oracleContainer.getUsername(),
+oracleContainer.getPassword(),
+OracleURLParser.parse(jdbcUrl),
+SCHEMA,
+null);
+catalog.open();
+}
+
+@AfterAll
+public static void stopContainer() throws SQLException {
+if (catalog != null) {
+catalog.close();
Review Comment:
Thank you for your contribution. If an exception is thrown when closing the
`catalog`, will the following be unable to close
##
seatunnel-connectors-v2/connector-jdbc/src/test/java/org/apache/seatunnel/connectors/seatunnel/jdbc/internal/dialect/oracle/container/OracleDialectContainerTest.java:
##
@@ -0,0 +1,216 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on a
Re: [PR] [Feat][Connector-v2][JDBC][Oracle] Add Testcontainers-based unit tests [seatunnel]
DanielCarter-stack commented on PR #10435:
URL: https://github.com/apache/seatunnel/pull/10435#issuecomment-3832926343
## Issue 1: Test method lacks assertions
**Location**: `OracleDialectContainerTest.java:172-183`
```java
@Test
public void testQueryNextChunkMax() throws Exception {
TablePath tablePath = TablePath.of(DATABASE, SCHEMA, TEST_TABLE);
JdbcSourceTable table =
JdbcSourceTable.builder()
.tablePath(tablePath)
.useSelectCount(false)
.skipAnalyze(false)
.build();
Object maxValue = dialect.queryNextChunkMax(connection, table, "ID", 10,
0);
}
```
**Related Context**:
- Reference: `DuckDBDialectTest.java:167-173` has complete assertions for
the same method
- Method under test: `OracleDialect.queryNextChunkMax()`
(OracleDialect.java:263-310)
**Problem Description**:
This test method calls `queryNextChunkMax()` but does not perform any
assertion verification on the return value. Referencing the same test in
`DuckDBDialectTest`, the returned maximum value should be verified as expected.
The current implementation will "pass" even if the method returns null or
throws an exception (unless the exception directly causes failure).
**Potential Risks**:
- Cannot verify the logic correctness of `queryNextChunkMax()`
- Cannot catch regression issues (e.g., future modifications breaking this
method)
- False high test coverage (method is called but not verified)
**Impact Scope**:
- Direct impact: `OracleDialect.queryNextChunkMax()` method
- Indirect impact: Oracle data source reading functionality that depends on
chunk sharding
- Affected area: Oracle Connector's data sharding logic
**Severity**: MAJOR
**Improvement Suggestions**:
```java
@Test
public void testQueryNextChunkMax() throws Exception {
TablePath tablePath = TablePath.of(DATABASE, SCHEMA, TEST_TABLE);
JdbcSourceTable table =
JdbcSourceTable.builder()
.tablePath(tablePath)
.useSelectCount(false)
.skipAnalyze(false)
.build();
// Test first chunk
Object firstChunkMax = dialect.queryNextChunkMax(connection, table,
"ID", 3, 1);
Assertions.assertNotNull(firstChunkMax);
Assertions.assertEquals(1, ((Number) firstChunkMax).intValue());
// Test second chunk
Object secondChunkMax = dialect.queryNextChunkMax(connection, table,
"ID", 3, 1);
Assertions.assertNotNull(secondChunkMax);
// Should return the only ID value (1) since table only has one row
Assertions.assertEquals(1, ((Number) secondChunkMax).intValue());
}
```
**Rationale**: Referencing the implementation of `DuckDBDialectTest`, the
return value should be verified as expected. Since only one row of data (ID=1)
was inserted in the test table, the chunk sharding should correctly handle this
case.
---
## Issue 2: Testcontainers version is outdated
**Location**: `pom.xml:431-439`
```xml
org.testcontainers
oracle-xe
${testcontainer.version}
test
```
**Related Context**:
- Parent POM definition: `testcontainer.version = 1.17.6` (pom.xml:141)
- Other modules in the project: connector-databend-e2e uses 1.20.2,
connector-hudi-e2e uses 1.19.1
**Problem Description**:
Testcontainers 1.17.6 was released in 2022 and is relatively old. Although
this version works, there are the following issues:
1. Lacks new features and bug fixes
2. May have known security issues
3. Inconsistent with versions used in other modules of the project (e.g.,
databend 1.20.2)
**Potential Risks**:
- Possible unfixed bugs affecting test stability
- Compatibility issues with newer Docker/Java versions
- Security vulnerabilities (although test dependencies don't affect
production, CI environment is still affected)
**Impact Scope**:
- Direct impact: Oracle tests in the connector-jdbc module
- Indirect impact: Test execution in CI environment
- Affected area: Test dependencies of a single module
**Severity**: MINOR
**Improvement Suggestions**:
Consider upgrading Testcontainers to version 1.19.x or 1.20.x to align with
other modules in the project. However, this requires:
1. Upgrading the `testcontainer.version` property in the parent POM
2. Verifying compatibility of all modules using Testcontainers
3. Handling in a separate issue, should not block the current PR
**Rationale**: Although the version is old, it does not affect
functionality. Suggested as a follow-up optimization item to be handled
together when upgrading Testcontainers version globally.
---
## Issue 3: Debug output should use logging frame
