XComp commented on code in PR #19915:
URL: https://github.com/apache/flink/pull/19915#discussion_r983556714
##########
flink-connectors/flink-connector-hbase-base/src/test/java/org/apache/flink/connector/hbase/util/HBaseSerdeTest.java:
##########
@@ -71,16 +69,15 @@ public void convertToNewRowTest() {
}
// this verifies RowData is not reused
- assertFalse(resultRowDatas.get(0) == resultRowDatas.get(1));
-
- List<String> expected = new ArrayList<>();
- expected.add("+I(1,+I(10),+I(Hello-1,100),+I(1.01,false,Welt-1))");
- expected.add("+I(2,+I(20),+I(Hello-2,200),+I(2.02,true,Welt-2))");
- assertEquals(expected, resultRowDataStr);
+ assertThat(resultRowDatas.get(0)).isNotSameAs(resultRowDatas.get(1));
Review Comment:
nit: I'm wondering whether we could make this part of the assert using the
`as()` method. The documentation would be present still but also included in
the assertion error output...
##########
flink-connectors/flink-connector-hbase-base/src/test/java/org/apache/flink/connector/hbase/util/HBaseConfigLoadingTest.java:
##########
@@ -64,13 +63,13 @@ public void loadFromEnvVariables() throws Exception {
final String k4 = "which way?";
final String v4 = "south, always south...";
- final File hbaseConfDir = tempFolder.newFolder();
+ final File hbaseConfDir = tmpDir.toFile();
- final File hbaseHome = tempFolder.newFolder();
+ final File hbaseHome = tmpDir.toFile();
Review Comment:
Don't we change the semantics here? The previous version created two folders
here. The new version uses the same folder for both files. :thinking: I guess
it would be saver to stick to separate folder here.
##########
flink-connectors/flink-connector-hbase-base/src/test/java/org/apache/flink/connector/hbase/util/HBaseSerdeTest.java:
##########
@@ -71,16 +69,15 @@ public void convertToNewRowTest() {
}
// this verifies RowData is not reused
- assertFalse(resultRowDatas.get(0) == resultRowDatas.get(1));
-
- List<String> expected = new ArrayList<>();
- expected.add("+I(1,+I(10),+I(Hello-1,100),+I(1.01,false,Welt-1))");
- expected.add("+I(2,+I(20),+I(Hello-2,200),+I(2.02,true,Welt-2))");
- assertEquals(expected, resultRowDataStr);
+ assertThat(resultRowDatas.get(0)).isNotSameAs(resultRowDatas.get(1));
Review Comment:
same applies in the `convertToReusedRowTest` test (line 92)
--
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]
