fapaul commented on code in PR #19302:
URL: https://github.com/apache/flink/pull/19302#discussion_r842598408


##########
flink-connectors/flink-connector-elasticsearch-base/src/test/java/org/apache/flink/streaming/connectors/elasticsearch/table/IndexGeneratorFactoryTest.java:
##########
@@ -197,8 +198,17 @@ public void 
testDynamicIndexDefaultFormatTimestampWithLocalTimeZone() {
         IndexGenerator indexGenerator =
                 
IndexGeneratorFactory.createIndexGenerator("my-index-{local_timestamp|}", 
schema);
         indexGenerator.open();
-        Assert.assertEquals("my-index-2020_03_18_12_12_14Z", 
indexGenerator.generate(rows.get(0)));
-        Assert.assertEquals("my-index-2020_03_19_12_12_14Z", 
indexGenerator.generate(rows.get(1)));
+        if (ZoneId.systemDefault().equals(ZoneId.of("UTC"))) {
+            Assert.assertEquals(
+                    "my-index-2020_03_18_12_12_14Z", 
indexGenerator.generate(rows.get(0)));
+            Assert.assertEquals(
+                    "my-index-2020_03_19_12_12_14Z", 
indexGenerator.generate(rows.get(1)));
+        } else {
+            Assert.assertNotEquals(
+                    "my-index-2020_03_18_12_12_14Z", 
indexGenerator.generate(rows.get(0)));
+            Assert.assertNotEquals(
+                    "my-index-2020_03_19_12_12_14Z", 
indexGenerator.generate(rows.get(1)));

Review Comment:
   The assertion looks wrong and both condition branch assert the same.



##########
flink-connectors/flink-connector-elasticsearch-base/src/test/java/org/apache/flink/connector/elasticsearch/table/IndexGeneratorTest.java:
##########
@@ -101,9 +105,37 @@
                                     LocalDateTime.of(2020, 3, 19, 12, 22, 14, 
1000)),
                             (int) LocalDate.of(2020, 3, 19).toEpochDay(),
                             (int) (LocalTime.of(12, 13, 14, 
2000).toNanoOfDay() / 1_000_000L),
+                            
TimestampData.fromTimestamp(Timestamp.valueOf("2020-03-19 12:22:14")),
                             "test2",
                             false));
 
+    @Test
+    public void testDynamicIndexFromTimestampTz() {
+        IndexGenerator indexGenerator =
+                IndexGeneratorFactory.createIndexGenerator(
+                        "{local_timestamp|yyyy_MM_dd_HH-ss}_index", 
fieldNames, dataTypes);
+        indexGenerator.open();
+        String index = indexGenerator.generate(rows.get(0));
+
+        if (ZoneId.systemDefault().equals(ZoneId.of("UTC"))) {
+            Assertions.assertEquals("2020_03_18_12-14_index", index);
+        } else {
+            Assertions.assertNotEquals("2020_03_18_12-14_index", index);
+        }
+
+        IndexGenerator indexGenerator1 =
+                IndexGeneratorFactory.createIndexGenerator(
+                        "{local_timestamp|yyyy_MM_dd_HH-ss}_index", 
fieldNames, dataTypes);
+        indexGenerator1.open();
+        String index1 = indexGenerator.generate(rows.get(0));
+
+        if (ZoneId.systemDefault().equals(ZoneId.of("UTC"))) {
+            Assertions.assertEquals("2020-03-19_12_22-14_index", index1);
+        } else {
+            Assertions.assertNotEquals("2020-03-19_12_22-14_index", index1);
+        }

Review Comment:
   The second part of the tests looks very similar to the first part, please 
extract a common method or better use a parameterized test to ensure both 
scenarios.



##########
flink-connectors/flink-connector-elasticsearch-base/src/test/java/org/apache/flink/connector/elasticsearch/table/IndexGeneratorTest.java:
##########
@@ -101,9 +105,37 @@
                                     LocalDateTime.of(2020, 3, 19, 12, 22, 14, 
1000)),
                             (int) LocalDate.of(2020, 3, 19).toEpochDay(),
                             (int) (LocalTime.of(12, 13, 14, 
2000).toNanoOfDay() / 1_000_000L),
+                            
TimestampData.fromTimestamp(Timestamp.valueOf("2020-03-19 12:22:14")),
                             "test2",
                             false));
 
+    @Test
+    public void testDynamicIndexFromTimestampTz() {
+        IndexGenerator indexGenerator =
+                IndexGeneratorFactory.createIndexGenerator(
+                        "{local_timestamp|yyyy_MM_dd_HH-ss}_index", 
fieldNames, dataTypes);
+        indexGenerator.open();

Review Comment:
   Nit: Do you also need to close the indexGenerator? Maybe you use a 
try-with-resource block 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