nsivabalan commented on a change in pull request #2421: URL: https://github.com/apache/hudi/pull/2421#discussion_r554629191
########## File path: hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/table/action/rollback/TestMarkerBasedRollbackStrategy.java ########## @@ -40,88 +48,103 @@ public class TestMarkerBasedRollbackStrategy extends HoodieClientTestBase { + private static final String TEST_NAME_WITH_PARAMS = "[{index}] Test with listing metadata enable={0}"; + + public static Stream<Arguments> configParams() { + return Arrays.stream(new Boolean[][] {{true}, {false}}).map(Arguments::of); + } + + private HoodieTableType tableType = HoodieTableType.COPY_ON_WRITE; + @BeforeEach public void setUp() throws Exception { initPath(); initSparkContexts(); initFileSystem(); - initMetaClient(); - initDFS(); + initMetaClient(tableType); + initTestDataGenerator(); } @AfterEach public void tearDown() throws Exception { cleanupResources(); } - @Test - public void testCopyOnWriteRollback() throws Exception { - // given: wrote some base files and corresponding markers - HoodieTestTable testTable = HoodieTestTable.of(metaClient); - String f0 = testTable.addRequestedCommit("000") - .getFileIdsWithBaseFilesInPartitions("partA").get("partA"); - String f1 = testTable.addCommit("001") - .withBaseFilesInPartition("partA", f0) - .getFileIdsWithBaseFilesInPartitions("partB").get("partB"); - String f2 = "f2"; - testTable.forCommit("001") - .withMarkerFile("partA", f0, IOType.MERGE) - .withMarkerFile("partB", f1, IOType.CREATE) - .withMarkerFile("partA", f2, IOType.CREATE); - - // when - List<HoodieRollbackStat> stats = new SparkMarkerBasedRollbackStrategy(HoodieSparkTable.create(getConfig(), context, metaClient), context, getConfig(), "002") - .execute(new HoodieInstant(HoodieInstant.State.INFLIGHT, HoodieTimeline.COMMIT_ACTION, "001")); - - // then: ensure files are deleted correctly, non-existent files reported as failed deletes - assertEquals(2, stats.size()); - - FileStatus[] partAFiles = testTable.listAllFilesInPartition("partA"); - FileStatus[] partBFiles = testTable.listAllFilesInPartition("partB"); - - assertEquals(0, partBFiles.length); - assertEquals(1, partAFiles.length); - assertEquals(2, stats.stream().mapToInt(r -> r.getSuccessDeleteFiles().size()).sum()); - assertEquals(1, stats.stream().mapToInt(r -> r.getFailedDeleteFiles().size()).sum()); + @ParameterizedTest(name = TEST_NAME_WITH_PARAMS) Review comment: I completely rewrote this test class. Existing one was leveraging HoodieTestTable which can be used for listing using FileSystem. So, rewrote to use writeClient to trigger commits and rollback and verify the files. ---------------------------------------------------------------- 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