wuchong commented on a change in pull request #12880:
URL: https://github.com/apache/flink/pull/12880#discussion_r472743692



##########
File path: 
flink-table/flink-table-api-java-bridge/src/test/java/org/apache/flink/table/api/bridge/java/internal/StreamTableEnvironmentImplTest.java
##########
@@ -52,18 +52,17 @@ public void testAppendStreamDoesNotOverwriteTableConfig() {
 
                StreamTableEnvironmentImpl tEnv = 
getStreamTableEnvironment(env, elements);
 
-               Time minRetention = Time.minutes(1);
-               Time maxRetention = Time.minutes(10);
-               tEnv.getConfig().setIdleStateRetentionTime(minRetention, 
maxRetention);
+               Duration minRetention = Duration.ofMinutes(1);
+               tEnv.getConfig().setIdleStateRetention(minRetention);
                Table table = tEnv.fromDataStream(elements);
                tEnv.toAppendStream(table, Row.class);
 
                assertThat(
                        tEnv.getConfig().getMinIdleStateRetentionTime(),
-                       equalTo(minRetention.toMilliseconds()));
+                       equalTo(minRetention.toMillis()));
                assertThat(
                        tEnv.getConfig().getMaxIdleStateRetentionTime(),
-                       equalTo(maxRetention.toMilliseconds()));
+                       equalTo(minRetention.toMillis() * 3 / 2));

Review comment:
       We only need to assert the `tEnv.getConfig().getIdleStateRetention()`.

##########
File path: 
flink-table/flink-table-api-java-bridge/src/test/java/org/apache/flink/table/api/bridge/java/internal/StreamTableEnvironmentImplTest.java
##########
@@ -73,18 +72,17 @@ public void testRetractStreamDoesNotOverwriteTableConfig() {
 
                StreamTableEnvironmentImpl tEnv = 
getStreamTableEnvironment(env, elements);
 
-               Time minRetention = Time.minutes(1);
-               Time maxRetention = Time.minutes(10);
-               tEnv.getConfig().setIdleStateRetentionTime(minRetention, 
maxRetention);
+               Duration minRetention = Duration.ofMinutes(1);
+               tEnv.getConfig().setIdleStateRetention(minRetention);
                Table table = tEnv.fromDataStream(elements);
                tEnv.toRetractStream(table, Row.class);
 
                assertThat(
                        tEnv.getConfig().getMinIdleStateRetentionTime(),
-                       equalTo(minRetention.toMilliseconds()));
+                       equalTo(minRetention.toMillis()));
                assertThat(
                        tEnv.getConfig().getMaxIdleStateRetentionTime(),
-                       equalTo(maxRetention.toMilliseconds()));
+                       equalTo(minRetention.toMillis() * 3 / 2));

Review comment:
       ditto.

##########
File path: 
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/api/TableConfigTest.java
##########
@@ -24,114 +24,54 @@
 
 import java.time.Duration;
 import java.time.ZoneId;
-import java.util.Arrays;
-import java.util.List;
-import java.util.function.BiConsumer;
-import java.util.function.Function;
 
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertEquals;
 
 /**
  * Tests for {@link TableConfig}.
  */
 public class TableConfigTest {
-       private static List<TestSpec> specs = Arrays.asList(
-               TestSpec.testValue(SqlDialect.HIVE)
-                       .viaSetter(TableConfig::setSqlDialect)
-                       .getterVia(TableConfig::getSqlDialect)
-                       .whenSetFromConfig("table.sql-dialect", "HIVE"),
-
-               TestSpec.testValue(5000)
-                       .viaSetter(TableConfig::setMaxGeneratedCodeLength)
-                       .getterVia(TableConfig::getMaxGeneratedCodeLength)
-                       .whenSetFromConfig("table.generated-code.max-length", 
"5000"),
-
-               TestSpec.testValue(ZoneId.of("Asia/Shanghai"))
-                       .viaSetter(TableConfig::setLocalTimeZone)
-                       .getterVia(TableConfig::getLocalTimeZone)
-                       .whenSetFromConfig("table.local-time-zone", 
"Asia/Shanghai"),
-
-               TestSpec.testValue(Duration.ofHours(1))
-                       .viaSetter(TableConfig::setIdleStateRetention)
-                       .getterVia(tableConfig -> 
Duration.ofMillis(tableConfig.getMinIdleStateRetentionTime()))
-                       .whenSetFromConfig("table.exec.state.ttl", "1 h"),
-
-               TestSpec.testValue(Duration.ofHours(2))
-                       .viaSetter(TableConfig::setIdleStateRetention)
-                       .getterVia(tableConfig -> 
Duration.ofMillis(tableConfig.getMaxIdleStateRetentionTime()))
-                       .whenSetFromConfig("table.exec.state.ttl", "2 h")
-                       .nonDefaultValue(Duration.ofHours(3))
-       );
+       private static TableConfig configByMethod = new TableConfig();
+       private static TableConfig configByConfiguration = new TableConfig();
+       private static Configuration configuration = new Configuration();
 
        @Test
-       public void testLoadFromConfiguration() {
-               for (TestSpec<?> spec: specs) {
-                       testWithSpec(spec);
-               }
-       }
-
-       private void testWithSpec(TestSpec<?> testSpec) {
-               Configuration config = new Configuration();
-               config.setString(testSpec.key, testSpec.value);
-               TableConfig tableConfigFromConfig = new TableConfig();
-               tableConfigFromConfig.addConfiguration(config);
+       public void testSetAndGetSqlDialect() {
+               configuration.setString("table.sql-dialect", "HIVE");
+               configByConfiguration.addConfiguration(configuration);
+               configByMethod.setSqlDialect(SqlDialect.HIVE);
 
-               TableConfig tableConfigFromSetter = new TableConfig();
-               testSpec.setValue(tableConfigFromSetter);
-
-               testSpec.assertEqual(tableConfigFromConfig, 
tableConfigFromSetter);
-               testSpec.assertEqualNonDefaultValue(tableConfigFromConfig);
+               assertEquals(SqlDialect.HIVE, configByMethod.getSqlDialect());
+               assertEquals(SqlDialect.HIVE, 
configByConfiguration.getSqlDialect());
        }
 
-       private static class TestSpec<T> {
-               private String key;
-               private String value;
-               private final T objectValue;
-               private BiConsumer<TableConfig, T> setter;
-               private Function<TableConfig, T> getter;
-               private T nonDefaultValue;
-
-               private TestSpec(T value) {
-                       this.objectValue = value;
-                       this.nonDefaultValue = value;
-               }
-
-               public static <T> TestSpec<T> testValue(T value) {
-                       return new TestSpec<>(value);
-               }
-
-               public TestSpec<T> nonDefaultValue(T value) {
-                       this.nonDefaultValue = value;
-                       return this;
-               }
-
-               public TestSpec<T> whenSetFromConfig(String key, String value) {
-                       this.key = key;
-                       this.value = value;
-                       return this;
-               }
+       @Test
+       public void testSetAndGetMaxGeneratedCodeLength() {
+               configuration.setString("table.generated-code.max-length", 
"5000");
+               configByConfiguration.addConfiguration(configuration);
+               configByMethod.setMaxGeneratedCodeLength(5000);
 
-               public TestSpec<T> viaSetter(BiConsumer<TableConfig, T> setter) 
{
-                       this.setter = setter;
-                       return this;
-               }
+               assertEquals(Integer.valueOf(5000), 
configByMethod.getMaxGeneratedCodeLength());
+               assertEquals(Integer.valueOf(5000), 
configByConfiguration.getMaxGeneratedCodeLength());
+       }
 
-               public TestSpec<T> getterVia(Function<TableConfig, T> getter) {
-                       this.getter = getter;
-                       return this;
-               }
+       @Test
+       public void testSetAndGetLocalTimeZone() {
+               configuration.setString("table.local-time-zone", 
"Asia/Shanghai");
+               configByConfiguration.addConfiguration(configuration);
+               configByMethod.setLocalTimeZone(ZoneId.of("Asia/Shanghai"));
 
-               public void setValue(TableConfig config) {
-                       setter.accept(config, objectValue);
-               }
+               assertEquals(Integer.valueOf(5000), 
configByMethod.getMaxGeneratedCodeLength());
+               assertEquals(Integer.valueOf(5000), 
configByConfiguration.getMaxGeneratedCodeLength());

Review comment:
       Should assert time zone. 




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to