Copilot commented on code in PR #7908:
URL: https://github.com/apache/incubator-seata/pull/7908#discussion_r2654712905
##########
rm-datasource/src/test/java/org/apache/seata/rm/datasource/sql/struct/TableRecordsTest.java:
##########
@@ -292,4 +294,81 @@ public void testEmpty() {
Assertions.assertThrows(UnsupportedOperationException.class, () ->
empty.add(new Row()));
Assertions.assertThrows(UnsupportedOperationException.class,
empty::getTableMeta);
}
+
+ private static Object[][] columnMetasOffsetDateTime = new Object[][] {
+ new Object[] {
+ "", "", "table_records_test", "id", Types.INTEGER, "INTEGER", 64,
0, 10, 1, "", "", 0, 0, 64, 1, "NO", "YES"
+ },
+ new Object[] {
+ "",
+ "",
+ "table_records_test",
+ "time_col",
+ Types.TIMESTAMP_WITH_TIMEZONE,
+ "TIMESTAMP_WITH_TIMEZONE",
+ 64,
+ 0,
+ 10,
+ 0,
+ "",
+ "",
+ 0,
+ 0,
+ 64,
+ 2,
+ "YES",
+ "NO"
+ },
+ };
+
+ private static List<String> returnValueColumnLabelsOffsetDateTime =
Lists.newArrayList("id", "time_col");
+
+ private static Object[][] returnValueOffsetDateTime = new Object[][] {
+ new Object[] {1, OffsetDateTime.of(2025, 1, 15, 10, 30, 45, 0,
ZoneOffset.UTC)},
+ };
+
+ @Test
+ public void testBuildRecordsWithOffsetDateTime() throws SQLException {
+ MockDriver mockDriver = new MockDriver(
+ returnValueColumnLabelsOffsetDateTime,
+ returnValueOffsetDateTime,
+ columnMetasOffsetDateTime,
+ indexMetas);
+
+ DruidDataSource dataSource = new DruidDataSource();
+ dataSource.setUrl("jdbc:mock:offset");
+ dataSource.setDriver(mockDriver);
+
+ try (MockStatementBase mockStatement = new
MockStatement(getPhysicsConnection(dataSource))) {
+ DataSourceProxy proxy =
DataSourceProxyTest.getDataSourceProxy(dataSource);
+ TableMetaCacheFactory.getTableMetaCache(JdbcConstants.MYSQL)
+ .refresh(proxy.getPlainConnection(),
proxy.getResourceId());
+ TableMeta tableMeta =
TableMetaCacheFactory.getTableMetaCache(JdbcConstants.MYSQL)
+ .getTableMeta(proxy.getPlainConnection(),
"table_records_test", proxy.getResourceId());
+ ResultSet originalResultSet =
mockDriver.executeQuery(mockStatement, "select * from table_records_test");
+ ResultSet proxyResultSet = (ResultSet)
java.lang.reflect.Proxy.newProxyInstance(
+ TableRecordsTest.class.getClassLoader(), new Class[]
{ResultSet.class}, (p, method, args) -> {
+ if ("getObject".equals(method.getName())
+ && args.length == 2
+ && args[1] == OffsetDateTime.class) {
+ return originalResultSet.getObject((Integer)
args[0]);
+ }
+ try {
+ return method.invoke(originalResultSet, args);
+ } catch (java.lang.reflect.InvocationTargetException
e) {
+ throw e.getTargetException();
+ }
+ });
+ TableRecords tableRecords = TableRecords.buildRecords(tableMeta,
proxyResultSet);
+ Assertions.assertNotNull(tableRecords);
+ Assertions.assertEquals(1, tableRecords.size());
+ Row row = tableRecords.getRows().get(0);
+ Field timeField = row.getFields().stream()
+ .filter(f -> "time_col".equalsIgnoreCase(f.getName()))
+ .findFirst()
+ .orElseThrow(() -> new RuntimeException("time_col not
found"));
+ Assertions.assertEquals(Types.TIMESTAMP_WITH_TIMEZONE,
timeField.getType());
+ Assertions.assertTrue(timeField.getValue() instanceof
OffsetDateTime);
+ }
+ }
Review Comment:
The test lacks coverage for the serialization and deserialization of
OffsetDateTime through the JacksonUndoLogParser. While the test verifies that
OffsetDateTime can be retrieved from the ResultSet and stored in a Field, it
doesn't test the critical path of serializing/deserializing this data through
the undo log, which is the primary use case affected by the JavaTimeModule
registration in JacksonUndoLogParser.
##########
rm-datasource/src/test/java/org/apache/seata/rm/datasource/sql/struct/TableRecordsTest.java:
##########
@@ -292,4 +294,81 @@ public void testEmpty() {
Assertions.assertThrows(UnsupportedOperationException.class, () ->
empty.add(new Row()));
Assertions.assertThrows(UnsupportedOperationException.class,
empty::getTableMeta);
}
+
+ private static Object[][] columnMetasOffsetDateTime = new Object[][] {
+ new Object[] {
+ "", "", "table_records_test", "id", Types.INTEGER, "INTEGER", 64,
0, 10, 1, "", "", 0, 0, 64, 1, "NO", "YES"
+ },
+ new Object[] {
+ "",
+ "",
+ "table_records_test",
+ "time_col",
+ Types.TIMESTAMP_WITH_TIMEZONE,
+ "TIMESTAMP_WITH_TIMEZONE",
+ 64,
+ 0,
+ 10,
+ 0,
+ "",
+ "",
+ 0,
+ 0,
+ 64,
+ 2,
+ "YES",
+ "NO"
+ },
+ };
+
+ private static List<String> returnValueColumnLabelsOffsetDateTime =
Lists.newArrayList("id", "time_col");
+
+ private static Object[][] returnValueOffsetDateTime = new Object[][] {
+ new Object[] {1, OffsetDateTime.of(2025, 1, 15, 10, 30, 45, 0,
ZoneOffset.UTC)},
+ };
+
+ @Test
+ public void testBuildRecordsWithOffsetDateTime() throws SQLException {
+ MockDriver mockDriver = new MockDriver(
+ returnValueColumnLabelsOffsetDateTime,
+ returnValueOffsetDateTime,
+ columnMetasOffsetDateTime,
+ indexMetas);
+
+ DruidDataSource dataSource = new DruidDataSource();
+ dataSource.setUrl("jdbc:mock:offset");
+ dataSource.setDriver(mockDriver);
+
+ try (MockStatementBase mockStatement = new
MockStatement(getPhysicsConnection(dataSource))) {
+ DataSourceProxy proxy =
DataSourceProxyTest.getDataSourceProxy(dataSource);
+ TableMetaCacheFactory.getTableMetaCache(JdbcConstants.MYSQL)
+ .refresh(proxy.getPlainConnection(),
proxy.getResourceId());
+ TableMeta tableMeta =
TableMetaCacheFactory.getTableMetaCache(JdbcConstants.MYSQL)
Review Comment:
The test is using JdbcConstants.MYSQL for a test that is specifically
designed to test PostgreSQL's TIMESTAMP_WITH_TIMEZONE handling. This is
misleading and could lead to incorrect test behavior. The test should use
JdbcConstants.POSTGRESQL instead to properly reflect the database being tested.
```suggestion
TableMetaCacheFactory.getTableMetaCache(JdbcConstants.POSTGRESQL)
.refresh(proxy.getPlainConnection(),
proxy.getResourceId());
TableMeta tableMeta =
TableMetaCacheFactory.getTableMetaCache(JdbcConstants.POSTGRESQL)
```
--
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]
