twalthr commented on code in PR #25704:
URL: https://github.com/apache/flink/pull/25704#discussion_r1863337407
##########
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/api/TableEnvironmentTest.java:
##########
@@ -49,55 +50,78 @@ class TableEnvironmentTest {
@Test
void testCreateTemporaryTableFromDescriptor() {
final TableEnvironmentMock tEnv =
TableEnvironmentMock.getStreamingInstance();
- final String catalog = tEnv.getCurrentCatalog();
- final String database = tEnv.getCurrentDatabase();
-
final Schema schema = Schema.newBuilder().column("f0",
DataTypes.INT()).build();
- tEnv.createTemporaryTable(
- "T",
-
TableDescriptor.forConnector("fake").schema(schema).option("a",
"Test").build());
- assertThat(
- tEnv.getCatalog(catalog)
- .orElseThrow(AssertionError::new)
- .tableExists(new ObjectPath(database, "T")))
- .isFalse();
+ assertTemporaryCreateTableFromDescriptor(tEnv, schema, false);
+ }
- final Optional<ContextResolvedTable> lookupResult =
- tEnv.getCatalogManager().getTable(ObjectIdentifier.of(catalog,
database, "T"));
- assertThat(lookupResult.isPresent()).isTrue();
+ @Test
+ void testCreateTemporaryTableIfNotExistsFromDescriptor() {
+ final TableEnvironmentMock tEnv =
TableEnvironmentMock.getStreamingInstance();
+ final Schema schema = Schema.newBuilder().column("f0",
DataTypes.INT()).build();
Review Comment:
this line exists 6 times in this class. introduce a
```
private static final Schema TEST_SCHEMA = Schema.newBuilder().column("f0",
DataTypes.INT()).build();
```
constant.
##########
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/api/TableEnvironmentTest.java:
##########
@@ -49,55 +50,78 @@ class TableEnvironmentTest {
@Test
void testCreateTemporaryTableFromDescriptor() {
final TableEnvironmentMock tEnv =
TableEnvironmentMock.getStreamingInstance();
- final String catalog = tEnv.getCurrentCatalog();
- final String database = tEnv.getCurrentDatabase();
-
final Schema schema = Schema.newBuilder().column("f0",
DataTypes.INT()).build();
- tEnv.createTemporaryTable(
- "T",
-
TableDescriptor.forConnector("fake").schema(schema).option("a",
"Test").build());
- assertThat(
- tEnv.getCatalog(catalog)
- .orElseThrow(AssertionError::new)
- .tableExists(new ObjectPath(database, "T")))
- .isFalse();
+ assertTemporaryCreateTableFromDescriptor(tEnv, schema, false);
+ }
- final Optional<ContextResolvedTable> lookupResult =
- tEnv.getCatalogManager().getTable(ObjectIdentifier.of(catalog,
database, "T"));
- assertThat(lookupResult.isPresent()).isTrue();
+ @Test
+ void testCreateTemporaryTableIfNotExistsFromDescriptor() {
+ final TableEnvironmentMock tEnv =
TableEnvironmentMock.getStreamingInstance();
+ final Schema schema = Schema.newBuilder().column("f0",
DataTypes.INT()).build();
- final CatalogBaseTable catalogTable =
lookupResult.get().getResolvedTable();
- assertThat(catalogTable instanceof CatalogTable).isTrue();
- assertThat(catalogTable.getUnresolvedSchema()).isEqualTo(schema);
-
assertThat(catalogTable.getOptions().get("connector")).isEqualTo("fake");
- assertThat(catalogTable.getOptions().get("a")).isEqualTo("Test");
+ assertTemporaryCreateTableFromDescriptor(tEnv, schema, true);
+ assertThatNoException()
+ .isThrownBy(
+ () ->
+ tEnv.createTemporaryTable(
+ "T",
+ TableDescriptor.forConnector("fake")
+ .schema(schema)
+ .option("a", "Test")
+ .build(),
Review Comment:
These lines exist 9 times in this class:
```suggestion
TableDescriptor.forConnector("fake")
.schema(schema)
.option("a", "Test")
.build(),
```
Introduce a:
```
private static final TableDescriptor TEST_DESCRIPTOR =
TableDescriptor.forConnector("fake")
.schema(schema)
.option("a", "Test")
.build()
```
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
