bvolpato commented on code in PR #39287:
URL: https://github.com/apache/beam/pull/39287#discussion_r3561556275


##########
sdks/java/io/iceberg/src/test/java/org/apache/beam/sdk/io/iceberg/BeamRowWrapperTest.java:
##########
@@ -85,6 +85,8 @@ public class BeamRowWrapperTest {
                   Types.NestedField.required(1, "nested_int", 
Types.IntegerType.get()))),
           Types.NestedField.required(14, "pass_through_field", 
Types.IntegerType.get()));
   private static final UUID TEST_UUID = UUID.randomUUID();
+  private static final Instant TEST_MICROS_INSTANT =
+      Instant.ofEpochSecond(1_609_459_200L, 123_456_000);

Review Comment:
   Keeping Instant.ofEpochSecond because nanosecond component explicitly 
exercises regression boundary: divisible by 1,000 but not 1,000,000. This 
matches existing timestamp precision tests in LogicalTypesTest.



##########
sdks/java/io/iceberg/src/test/java/org/apache/beam/sdk/io/iceberg/BeamRowWrapperTest.java:
##########
@@ -180,10 +182,7 @@ public void testDateTimeConversion() {
 
   @Test
   public void testMicrosInstantLogicalTypeConversion() {
-    Instant javaInstant = ROW.getLogicalTypeValue(7, Instant.class);
-    long expectedMicrosInstant =
-        TimeUnit.SECONDS.toMicros(javaInstant.getEpochSecond()) + 
javaInstant.getNano() / 1000;
-    assertEquals(expectedMicrosInstant, (long) WRAPPER.get(7, Long.class));
+    assertEquals(1_609_459_200_123_456L, (long) WRAPPER.get(7, Long.class));

Review Comment:
   Keeping primitive cast to select JUnit assertEquals(long, long) overload and 
match neighboring conversion assertions in this test class. Unexpected null is 
a fixture contract failure here.



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