gemini-code-assist[bot] commented on code in PR #37775:
URL: https://github.com/apache/beam/pull/37775#discussion_r2890344867
##########
sdks/java/io/influxdb/src/test/java/org/apache/beam/sdk/io/influxdb/GenerateData.java:
##########
@@ -40,7 +40,7 @@ static List<String> getMetric(String metric, int number) {
m.addField("field2", random.nextInt(100));
LocalDateTime time =
LocalDateTime.of(
- LocalDate.now(),
+ LocalDate.now(ZoneId.systemDefault()),
Review Comment:
The current date is being fetched using the system's default timezone, but
then it's being combined with a random time and interpreted in the
`America/Los_Angeles` timezone. This can lead to incorrect timestamps,
especially around date boundaries. To ensure correctness, the date should be
obtained directly in the `America/Los_Angeles` timezone.
```suggestion
LocalDate.now(ZoneId.of("America/Los_Angeles")),
```
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/bigquery/BeamRowToStorageApiProtoTest.java:
##########
@@ -253,9 +254,9 @@ public class BeamRowToStorageApiProtoTest {
.withFieldValue("arrayValue", ImmutableList.of("one", "two", "red",
"blue"))
.withFieldValue("arrayNullValue", null)
.withFieldValue("iterableValue", ImmutableList.of("blue", "red",
"two", "one"))
- .withFieldValue("sqlDateValue", LocalDate.now())
- .withFieldValue("sqlTimeValue", LocalTime.now())
- .withFieldValue("sqlDatetimeValue", LocalDateTime.now())
+ .withFieldValue("sqlDateValue",
LocalDate.now(ZoneId.systemDefault()))
+ .withFieldValue("sqlTimeValue",
LocalTime.now(ZoneId.systemDefault()))
+ .withFieldValue("sqlDatetimeValue",
LocalDateTime.now(ZoneId.systemDefault()))
Review Comment:
Using `ZoneId.systemDefault()` to initialize a static test data field can
lead to flaky tests that depend on the timezone of the execution environment.
It's better to use a fixed timezone, like `ZoneId.of("UTC")`, to ensure
deterministic test data.
```suggestion
.withFieldValue("sqlDateValue", LocalDate.now(ZoneId.of("UTC")))
.withFieldValue("sqlTimeValue", LocalTime.now(ZoneId.of("UTC")))
.withFieldValue("sqlDatetimeValue",
LocalDateTime.now(ZoneId.of("UTC")))
```
##########
sdks/java/extensions/sql/src/test/java/org/apache/beam/sdk/extensions/sql/impl/schema/BeamSqlRowCoderTest.java:
##########
@@ -70,9 +71,9 @@ public void encodeAndDecode() throws Exception {
1.1,
BigDecimal.ZERO,
"hello",
- LocalTime.now(),
- LocalDate.now(),
- LocalDateTime.now(),
+ LocalTime.now(ZoneId.systemDefault()),
+ LocalDate.now(ZoneId.systemDefault()),
+ LocalDateTime.now(ZoneId.systemDefault()),
Review Comment:
Using `ZoneId.systemDefault()` can make tests flaky as they depend on the
timezone of the machine running the test. For better test stability and
reproducibility, it's recommended to use a fixed timezone, such as
`ZoneId.of("UTC")`.
```suggestion
LocalTime.now(ZoneId.of("UTC")),
LocalDate.now(ZoneId.of("UTC")),
LocalDateTime.now(ZoneId.of("UTC")),
```
##########
sdks/java/io/influxdb/src/test/java/org/apache/beam/sdk/io/influxdb/GenerateData.java:
##########
@@ -64,7 +64,7 @@ static List<String> getMultipleMetric(List<String> metrics,
int number) {
m.addField("field2", random.nextInt(100));
LocalDateTime time =
LocalDateTime.of(
- LocalDate.now(),
+ LocalDate.now(ZoneId.systemDefault()),
Review Comment:
Similar to the other instance in this file, the current date is being
fetched using the system's default timezone and then interpreted in
`America/Los_Angeles`. This can lead to incorrect data. Please fetch the date
using the `America/Los_Angeles` timezone directly.
```suggestion
LocalDate.now(ZoneId.of("America/Los_Angeles")),
```
##########
sdks/java/extensions/sbe/src/test/java/org/apache/beam/sdk/extensions/sbe/SbeLogicalTypesTest.java:
##########
@@ -59,7 +59,7 @@ public void testTzTimestamp() {
@Test
public void testUtcTimeOnly() {
- LocalTime time = LocalTime.now();
+ LocalTime time = LocalTime.now(ZoneId.systemDefault());
Review Comment:
Using `ZoneId.systemDefault()` can make tests flaky as they depend on the
timezone of the machine running the test. For better test stability and
reproducibility, it's recommended to use a fixed timezone, such as
`ZoneId.of("UTC")`, especially since the class under test is `UTCTimeOnly`.
```suggestion
LocalTime time = LocalTime.now(ZoneId.of("UTC"));
```
##########
sdks/java/extensions/sbe/src/test/java/org/apache/beam/sdk/extensions/sbe/SbeLogicalTypesTest.java:
##########
@@ -89,7 +89,7 @@ public void testUtcDateOnly() {
@Test
public void testLocalMktDate() {
- LocalDate date = LocalDate.now();
+ LocalDate date = LocalDate.now(ZoneId.systemDefault());
Review Comment:
Using `ZoneId.systemDefault()` can make tests flaky. For better test
stability, please use a fixed timezone like `ZoneId.of("UTC")`.
```suggestion
LocalDate date = LocalDate.now(ZoneId.of("UTC"));
```
##########
sdks/java/io/azure/src/test/java/org/apache/beam/sdk/io/azure/blobstore/AzureBlobStoreFileSystemTest.java:
##########
@@ -111,7 +111,8 @@ public void beforeClass() {
when(mockedBlobClient.getBlockBlobClient()).thenReturn(mockedBlockBlob);
when(mockedBlobClient.getProperties()).thenReturn(mockedProperties);
when(mockedProperties.getBlobSize()).thenReturn(Long.valueOf(1));
- when(mockedProperties.getLastModified()).thenReturn(OffsetDateTime.now());
+ when(mockedProperties.getLastModified())
+ .thenReturn(OffsetDateTime.now(java.time.ZoneId.systemDefault()));
Review Comment:
Using `ZoneId.systemDefault()` can make tests flaky. For better test
stability, please use a fixed timezone like UTC. You can use
`java.time.ZoneOffset.UTC`.
```suggestion
when(mockedProperties.getLastModified())
.thenReturn(OffsetDateTime.now(java.time.ZoneOffset.UTC));
```
##########
sdks/java/extensions/sbe/src/test/java/org/apache/beam/sdk/extensions/sbe/SbeLogicalTypesTest.java:
##########
@@ -79,7 +79,7 @@ public void testTzTimeOnly() {
@Test
public void testUtcDateOnly() {
- LocalDate date = LocalDate.now();
+ LocalDate date = LocalDate.now(ZoneId.systemDefault());
Review Comment:
Using `ZoneId.systemDefault()` can make tests flaky. For better test
stability, please use a fixed timezone like `ZoneId.of("UTC")`, especially
given the class under test is `UTCDateOnly`.
```suggestion
LocalDate date = LocalDate.now(ZoneId.of("UTC"));
```
##########
sdks/java/io/elasticsearch-tests/elasticsearch-tests-common/src/main/java/org/apache/beam/sdk/io/elasticsearch/ElasticsearchIOTestUtils.java:
##########
@@ -343,7 +343,7 @@ static long refreshIndexAndGetCurrentNumDocs(
static List<String> createDocuments(long numDocs, InjectionMode
injectionMode) {
ArrayList<String> data = new ArrayList<>();
- LocalDateTime baseDateTime = LocalDateTime.now();
+ LocalDateTime baseDateTime =
LocalDateTime.now(java.time.ZoneId.systemDefault());
Review Comment:
Using `ZoneId.systemDefault()` can make tests flaky as they depend on the
timezone of the machine running the test. For better test stability, please use
a fixed timezone like UTC. You can use `java.time.ZoneOffset.UTC`.
```suggestion
LocalDateTime baseDateTime = LocalDateTime.now(java.time.ZoneOffset.UTC);
```
--
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]
