You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2020/09/17 20:26:58 UTC
[GitHub] [hudi] xushiyan commented on a change in pull request #2094: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
xushiyan commented on a change in pull request #2094:
URL: https://github.com/apache/hudi/pull/2094#discussion_r490540439
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestTable.java
##########
@@ -268,16 +292,16 @@ public boolean baseFileExists(String partition, String instantTime, String fileI
}
}
- public Path getPartitionPath(String partition) {
- return new Path(Paths.get(basePath, partition).toUri());
- }
-
- public String getBaseFileNameById(String fileId) {
- return baseFileName(currentInstantTime, fileId);
+ public boolean logFilesExist(String partition, String instantTime, String fileId, int... versions) {
+ return Arrays.stream(versions).allMatch(v -> logFileExists(partition, instantTime, fileId, v));
}
- public Path getBaseFilePath(String partition, String fileId) {
- return new Path(Paths.get(basePath, partition, getBaseFileNameById(fileId)).toUri());
+ public boolean logFileExists(String partition, String instantTime, String fileId, int version) {
+ try {
+ return fs.exists(new Path(Paths.get(basePath, partition, logFileName(instantTime, fileId, version)).toString()));
+ } catch (IOException e) {
+ throw new HoodieTestTableException(e);
+ }
Review comment:
just reordering the APIs. API order is sth like inflight commit, commit, basefile, logfile,
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/CompactionTestUtils.java
##########
@@ -195,11 +195,11 @@ public static HoodieCompactionPlan createCompactionPlan(HoodieTableMetaClient me
/**
* The hoodie data file for testing.
*/
- public static class TestHoodieBaseFile extends HoodieBaseFile {
+ public static class DummyHoodieBaseFile extends HoodieBaseFile {
Review comment:
Class that is not an actual test should not start with `Test`
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestTable.java
##########
@@ -292,16 +316,16 @@ public Path getRequestedCompactionFilePath(String instantTime) {
return new Path(Paths.get(basePath, HoodieTableMetaClient.AUXILIARYFOLDER_NAME, instantTime + HoodieTimeline.REQUESTED_COMPACTION_EXTENSION).toUri());
}
- public boolean logFilesExist(String partition, String instantTime, String fileId, int... versions) {
- return Arrays.stream(versions).allMatch(v -> logFileExists(partition, instantTime, fileId, v));
+ public Path getPartitionPath(String partition) {
+ return new Path(Paths.get(basePath, partition).toUri());
}
- public boolean logFileExists(String partition, String instantTime, String fileId, int version) {
- try {
- return fs.exists(new Path(Paths.get(basePath, partition, logFileName(instantTime, fileId, version)).toString()));
- } catch (IOException e) {
- throw new HoodieTestTableException(e);
- }
+ public Path getBaseFilePath(String partition, String fileId) {
+ return new Path(Paths.get(basePath, partition, getBaseFileNameById(fileId)).toUri());
+ }
+
+ public String getBaseFileNameById(String fileId) {
+ return baseFileName(currentInstantTime, fileId);
Review comment:
ditto
##########
File path: hudi-client/src/test/java/org/apache/hudi/table/action/rollback/TestCopyOnWriteRollbackActionExecutor.java
##########
@@ -94,34 +89,40 @@ public void testCopyOnWriteRollbackActionExecutorForFileListingAsGenerateFile()
// assert hoodieRollbackStats
assertEquals(hoodieRollbackStats.size(), 3);
- hoodieRollbackStats.forEach(stat -> {
- if (stat.getPartitionPath().equals("2015/03/16")) {
- assertEquals(1, stat.getSuccessDeleteFiles().size());
- assertEquals(0, stat.getFailedDeleteFiles().size());
- assertEquals(Collections.EMPTY_MAP, stat.getCommandBlocksCount());
- assertEquals("file:" + HoodieTestUtils.getDataFilePath(basePath, "2015/03/16", commitTime2, file21),
- stat.getSuccessDeleteFiles().get(0));
- } else if (stat.getPartitionPath().equals("2015/03/17")) {
- assertEquals(1, stat.getSuccessDeleteFiles().size());
- assertEquals(0, stat.getFailedDeleteFiles().size());
- assertEquals(Collections.EMPTY_MAP, stat.getCommandBlocksCount());
- assertEquals("file:" + HoodieTestUtils.getDataFilePath(basePath, "2015/03/17", commitTime2, file22),
- stat.getSuccessDeleteFiles().get(0));
- } else if (stat.getPartitionPath().equals("2016/03/15")) {
- assertEquals(0, stat.getSuccessDeleteFiles().size());
- assertEquals(0, stat.getFailedDeleteFiles().size());
- assertEquals(Collections.EMPTY_MAP, stat.getCommandBlocksCount());
+ for (HoodieRollbackStat stat : hoodieRollbackStats) {
+ switch (stat.getPartitionPath()) {
+ case p1:
+ assertEquals(1, stat.getSuccessDeleteFiles().size());
+ assertEquals(0, stat.getFailedDeleteFiles().size());
+ assertEquals(Collections.EMPTY_MAP, stat.getCommandBlocksCount());
+ assertEquals(testTable.forCommit("002").getBaseFilePath(p1, "id21").toString(),
+ stat.getSuccessDeleteFiles().get(0));
+ break;
+ case p2:
+ assertEquals(1, stat.getSuccessDeleteFiles().size());
+ assertEquals(0, stat.getFailedDeleteFiles().size());
+ assertEquals(Collections.EMPTY_MAP, stat.getCommandBlocksCount());
+ assertEquals(testTable.forCommit("002").getBaseFilePath(p2, "id22").toString(),
+ stat.getSuccessDeleteFiles().get(0));
+ break;
+ case p3:
+ assertEquals(0, stat.getSuccessDeleteFiles().size());
+ assertEquals(0, stat.getFailedDeleteFiles().size());
+ assertEquals(Collections.EMPTY_MAP, stat.getCommandBlocksCount());
+ break;
+ default:
+ fail("Unexpected partition: " + stat.getPartitionPath());
Review comment:
change to `swtich` for case completeness
----------------------------------------------------------------
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:
users@infra.apache.org