Copilot commented on code in PR #954:
URL: https://github.com/apache/fesod/pull/954#discussion_r3607528898
##########
fesod-sheet/src/test/java/org/apache/fesod/sheet/format/CsvRowTest.java:
##########
@@ -120,6 +125,88 @@ void testCsvWriteWithModelShouldSuccess() {
.doWrite(modelData());
}
+ /**
+ * Verifies that {@link CsvCell} handles {@link java.sql.Date} the same
way as
+ * {@link org.apache.fesod.sheet.metadata.data.WriteCellData}: the date is
extracted
+ * via {@code toLocalDate().atStartOfDay()}, stripping any time component
that may
+ * exist in the underlying milliseconds (common when JDBC drivers create
+ * {@code java.sql.Date} from a {@code java.util.Date} with time info).
+ */
+ @Test
+ void testCsvCellSqlDateConversion() {
+ // Create a java.sql.Date from a java.util.Date that has a time
component
+ Calendar cal = Calendar.getInstance();
+ cal.set(2023, Calendar.JUNE, 15, 23, 30, 0);
+ cal.set(Calendar.MILLISECOND, 0);
+ java.sql.Date sqlDate = new java.sql.Date(cal.getTimeInMillis());
+
+ Cell cell = csvRow.createCell(0, CellType.NUMERIC);
+ cell.setCellValue(sqlDate);
+
+ LocalDateTime dateValue = ((CsvCell) cell).getLocalDateTimeCellValue();
+ // java.sql.Date is date-only: time component must be stripped to
midnight
+ Assertions.assertEquals(LocalDateTime.of(2023, 6, 15, 0, 0, 0),
dateValue);
+ }
+
+ /**
+ * Verifies that {@link CsvCell} handles {@link java.sql.Time} the same
way as
+ * {@link org.apache.fesod.sheet.metadata.data.WriteCellData}: the time is
extracted
+ * via {@code toLocalTime().atDate(LocalDate.of(1970, 1, 1))}, stripping
any date
+ * component that may exist in the underlying milliseconds.
+ */
+ @Test
+ void testCsvCellSqlTimeConversion() {
+ // Create a java.sql.Time from a java.util.Date that has a date
component
+ Calendar cal = Calendar.getInstance();
+ cal.set(2023, Calendar.JUNE, 15, 12, 30, 45);
+ cal.set(Calendar.MILLISECOND, 0);
+ java.sql.Time sqlTime = new java.sql.Time(cal.getTimeInMillis());
+
+ Cell cell = csvRow.createCell(0, CellType.NUMERIC);
+ cell.setCellValue(sqlTime);
+
+ LocalDateTime dateValue = ((CsvCell) cell).getLocalDateTimeCellValue();
+ // java.sql.Time is time-only: date component must be normalized to
1970-01-01
+ Assertions.assertEquals(LocalDateTime.of(1970, 1, 1, 12, 30, 45),
dateValue);
Review Comment:
This test hard-codes the expected LocalDateTime (including the time
component) based on Calendar.getInstance(), which depends on the default JVM
timezone. java.sql.Time.toLocalTime() is derived from the underlying epoch
millis and can vary with how those millis were constructed in different
timezones, making this assertion flaky. Derive the expected value from sqlTime
itself (and assert the call doesn’t throw) to keep the test stable while still
verifying the intended conversion path.
##########
fesod-sheet/src/test/java/org/apache/fesod/sheet/format/CsvRowTest.java:
##########
@@ -120,6 +125,88 @@ void testCsvWriteWithModelShouldSuccess() {
.doWrite(modelData());
}
+ /**
+ * Verifies that {@link CsvCell} handles {@link java.sql.Date} the same
way as
+ * {@link org.apache.fesod.sheet.metadata.data.WriteCellData}: the date is
extracted
+ * via {@code toLocalDate().atStartOfDay()}, stripping any time component
that may
+ * exist in the underlying milliseconds (common when JDBC drivers create
+ * {@code java.sql.Date} from a {@code java.util.Date} with time info).
+ */
+ @Test
+ void testCsvCellSqlDateConversion() {
+ // Create a java.sql.Date from a java.util.Date that has a time
component
+ Calendar cal = Calendar.getInstance();
+ cal.set(2023, Calendar.JUNE, 15, 23, 30, 0);
+ cal.set(Calendar.MILLISECOND, 0);
+ java.sql.Date sqlDate = new java.sql.Date(cal.getTimeInMillis());
+
+ Cell cell = csvRow.createCell(0, CellType.NUMERIC);
+ cell.setCellValue(sqlDate);
+
+ LocalDateTime dateValue = ((CsvCell) cell).getLocalDateTimeCellValue();
+ // java.sql.Date is date-only: time component must be stripped to
midnight
+ Assertions.assertEquals(LocalDateTime.of(2023, 6, 15, 0, 0, 0),
dateValue);
Review Comment:
These assertions hard-code expected LocalDateTime values derived from a
Calendar in the default JVM timezone. The underlying millis ->
java.sql.Date/toLocalDate conversion is timezone-sensitive, so this can become
flaky when tests run under different TZ settings. Prefer deriving the expected
value from sqlDate itself (and assert the call doesn’t throw) to keep the test
deterministic while still catching the pre-fix UnsupportedOperationException.
--
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]
