wombatu-kun commented on code in PR #19877:
URL: https://github.com/apache/hudi/pull/19877#discussion_r3974934958


##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestCompactionCommand.java:
##########
@@ -146,6 +173,203 @@ public void testCompactionShow() throws IOException {
     assertNotNull(result);
   }
 
+  /**
+   * Test case of the compaction validation entry point of {@link SparkMain}, 
which the
+   * 'compaction validate' command reaches through a spark-submit of its own.
+   */
+  @Test
+  public void testSparkMainCompactValidate() throws Exception {
+    createPendingCompactions();
+    String outputPath = outputPath("validate");
+
+    SparkMain.doCompactValidate(jsc(), tablePath, PENDING_COMPACTION_INSTANT, 
outputPath, 2);
+
+    List<ValidationOpResult> results = readOperationResults(outputPath);
+    assertEquals(operationsOf(PENDING_COMPACTION_INSTANT).size(), 
results.size());
+    assertTrue(results.stream().allMatch(ValidationOpResult::isSuccess), 
results.toString());
+    assertEquals(fileIdsOf(PENDING_COMPACTION_INSTANT),
+        results.stream().map(result -> 
result.getOperation().getFileId()).collect(Collectors.toSet()));
+  }
+
+  @Test
+  public void testSparkMainCompactValidateReportsMissingLogFile() throws 
Exception {
+    createPendingCompactions();
+    HoodieCompactionOperation broken = 
operationsOf(PENDING_COMPACTION_INSTANT).get(0);
+    // a log file the plan reads is gone, so that operation can no longer be 
compacted
+    Files.delete(Paths.get(tablePath, broken.getPartitionPath(), 
broken.getDeltaFilePaths().get(0)));
+    String outputPath = outputPath("validate-broken");
+
+    SparkMain.doCompactValidate(jsc(), tablePath, PENDING_COMPACTION_INSTANT, 
outputPath, 2);
+
+    List<ValidationOpResult> results = readOperationResults(outputPath);
+    assertEquals(operationsOf(PENDING_COMPACTION_INSTANT).size(), 
results.size());
+    List<ValidationOpResult> failed = results.stream().filter(result -> 
!result.isSuccess()).collect(Collectors.toList());
+    assertEquals(1, failed.size(), results.toString());
+    assertEquals(broken.getFileId(), failed.get(0).getOperation().getFileId());
+    assertTrue(failed.get(0).getException().isPresent());
+  }
+
+  /**
+   * Repair only validates the plan and reports the renames it would need; 
with the plan intact

Review Comment:
   `CompactionAdminClient.repairCompaction` discards the validation result and 
returns an empty list without ever reading `dryRun`, so repair never reports 
the renames it would need and the two parameterized arms are the same run. 
Could the comment drop that claim and the parameterization go?



##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestMetadataCommand.java:
##########
@@ -224,6 +235,222 @@ public void testGetRecordIndexInfoForPartitionedRLI() 
throws Exception {
     }
   }
 
+  @Test
+  public void testMetadataStatsAndFileListing() throws Exception {
+    writeOneCommit(true);
+    connectToTable();
+
+    // The command opens the reader with metadata metrics off, so there is 
nothing to report on,
+    // but the stat table is still rendered.
+    Object stats = shell.evaluate(() -> "metadata stats");
+    assertTrue(ShellEvaluationResultUtil.isSuccess(stats));
+    assertTrue(stats.toString().contains("stat key"), stats.toString());
+    assertTrue(renderedRows(stats.toString()).isEmpty(), stats.toString());
+
+    Object partitions = shell.evaluate(() -> "metadata list-partitions");
+    assertTrue(ShellEvaluationResultUtil.isSuccess(partitions));
+    Set<String> written = writtenPartitions();
+    assertFalse(written.isEmpty());
+    assertEquals(written, renderedRows(partitions.toString()).stream()
+        .map(row -> row.get(0)).collect(Collectors.toSet()));
+
+    // The files of one partition, as the metadata table has them.
+    Object files = shell.evaluate(() -> "metadata list-files --partition " + 
DEFAULT_FIRST_PARTITION_PATH);
+    assertTrue(ShellEvaluationResultUtil.isSuccess(files));
+    Set<String> baseFiles = baseFilesOf(DEFAULT_FIRST_PARTITION_PATH);
+    assertFalse(baseFiles.isEmpty());
+    assertEquals(baseFiles.size(), renderedRows(files.toString()).size(), 
files.toString());
+    for (String baseFile : baseFiles) {
+      assertTrue(files.toString().contains(baseFile), files.toString());
+    }
+
+    // Without a partition the base path itself is listed, which holds no data 
files.

Review Comment:
   Nothing is listed here: with no partition the base path resolves to the 
non-partitioned record key, and a partitioned table's files index holds no 
record under it, so the lookup misses rather than listing the directory. Could 
the comment say that instead?



##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestRollbacksCommand.java:
##########
@@ -205,4 +208,24 @@ public void testShowRollback() throws IOException {
     String got = removeNonWordAndStripSpace(result.toString());
     assertEquals(expected, got);
   }
+
+  /**
+   * Test case of the rollback entry point of {@link SparkMain}, which the 
'commit rollback'
+   * command reaches through a spark-submit of its own. The fixture leaves 
commit 101 in the
+   * inflight state, which is the failed write such a rollback is meant to 
clean up.
+   */
+  @Test
+  public void testSparkMainRollback() throws Exception {
+    HoodieTableMetaClient metaClient = HoodieCLI.getTableMetaClient();
+    HoodieActiveTimeline timeline = metaClient.reloadActiveTimeline();
+    
assertTrue(timeline.getCommitsTimeline().filterInflightsAndRequested().containsInstant("101"));
+    int rollbacksBefore = 
timeline.getRollbackTimeline().filterCompletedInstants().countInstants();
+
+    assertEquals(0, SparkMain.rollback(jsc(), "101", tablePath, false));

Review Comment:
   Only the success arm of `SparkMain.rollback` runs here; an instant that is 
not on the timeline makes `client.rollback` return false and the helper return 
-1, and nothing covers that. Could this add the not-found case, the way the 
savepoint test pins the -1 arms of `createSavepoint` and `rollbackToSavepoint`?



##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestClusteringCommand.java:
##########
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.commands;
+
+import org.apache.hudi.cli.HoodieCLI;
+import org.apache.hudi.cli.functional.CLIFunctionalTestHarness;
+import org.apache.hudi.client.SparkRDDWriteClient;
+import org.apache.hudi.client.WriteStatus;
+import org.apache.hudi.common.model.HoodieAvroPayload;
+import org.apache.hudi.common.model.HoodieRecord;
+import org.apache.hudi.common.model.HoodieTableType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.HoodieTableVersion;
+import org.apache.hudi.common.testutils.HoodieTestDataGenerator;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.testutils.Assertions;
+import org.apache.hudi.utilities.UtilHelpers;
+
+import org.apache.spark.api.java.JavaRDD;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.springframework.boot.test.context.SpringBootTest;
+
+import java.io.IOException;
+import java.util.Collections;
+import java.util.List;
+
+import static 
org.apache.hudi.common.testutils.HoodieTestDataGenerator.DEFAULT_FIRST_PARTITION_PATH;
+import static 
org.apache.hudi.common.testutils.HoodieTestDataGenerator.TRIP_EXAMPLE_SCHEMA;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * Test cases for the clustering entry point of {@link SparkMain}, which the 
clustering commands
+ * reach through a spark-submit of their own.
+ */
+@Tag("functional")
+@SpringBootTest(properties = {"spring.shell.interactive.enabled=false", 
"spring.shell.command.script.enabled=false"})
+public class TestClusteringCommand extends CLIFunctionalTestHarness {
+
+  private String tableName;
+  private String tablePath;
+
+  @BeforeEach
+  public void init() throws IOException {
+    HoodieCLI.conf = storageConf();
+    tableName = tableName();
+    tablePath = tablePath(tableName);
+
+    new TableCommand().createTable(
+        tablePath, tableName, HoodieTableType.COPY_ON_WRITE.name(),
+        "", HoodieTableVersion.current().versionCode(), 
HoodieAvroPayload.class.getName());
+  }
+
+  @Test
+  public void testSparkMainClusterScheduleAndExecute() throws Exception {
+    writeCommits();
+    HoodieTableMetaClient metaClient = 
HoodieTableMetaClient.reload(HoodieCLI.getTableMetaClient());
+    assertEquals(0, 
metaClient.getActiveTimeline().filterPendingClusteringTimeline().countInstants());
+
+    int returnCode = SparkMain.cluster(jsc(), tablePath, tableName, null, 1, 
"1g", 0,
+        UtilHelpers.SCHEDULE_AND_EXECUTE, null,
+        Collections.singletonList("hoodie.clustering.inline.max.commits=1"));

Review Comment:
   `ClusteringPlanActionExecutor` reads `hoodie.clustering.inline.max.commits` 
only when `hoodie.clustering.inline` is true, which it is not here, so this 
override is never consulted and the plan comes purely from the size-based 
strategy. Could it go, or is `hoodie.clustering.inline=true` meant to come with 
it?



##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestCompactionCommand.java:
##########
@@ -146,6 +173,203 @@ public void testCompactionShow() throws IOException {
     assertNotNull(result);
   }
 
+  /**
+   * Test case of the compaction validation entry point of {@link SparkMain}, 
which the
+   * 'compaction validate' command reaches through a spark-submit of its own.
+   */
+  @Test
+  public void testSparkMainCompactValidate() throws Exception {
+    createPendingCompactions();
+    String outputPath = outputPath("validate");
+
+    SparkMain.doCompactValidate(jsc(), tablePath, PENDING_COMPACTION_INSTANT, 
outputPath, 2);
+
+    List<ValidationOpResult> results = readOperationResults(outputPath);
+    assertEquals(operationsOf(PENDING_COMPACTION_INSTANT).size(), 
results.size());
+    assertTrue(results.stream().allMatch(ValidationOpResult::isSuccess), 
results.toString());
+    assertEquals(fileIdsOf(PENDING_COMPACTION_INSTANT),
+        results.stream().map(result -> 
result.getOperation().getFileId()).collect(Collectors.toSet()));
+  }
+
+  @Test
+  public void testSparkMainCompactValidateReportsMissingLogFile() throws 
Exception {
+    createPendingCompactions();
+    HoodieCompactionOperation broken = 
operationsOf(PENDING_COMPACTION_INSTANT).get(0);
+    // a log file the plan reads is gone, so that operation can no longer be 
compacted
+    Files.delete(Paths.get(tablePath, broken.getPartitionPath(), 
broken.getDeltaFilePaths().get(0)));
+    String outputPath = outputPath("validate-broken");
+
+    SparkMain.doCompactValidate(jsc(), tablePath, PENDING_COMPACTION_INSTANT, 
outputPath, 2);
+
+    List<ValidationOpResult> results = readOperationResults(outputPath);
+    assertEquals(operationsOf(PENDING_COMPACTION_INSTANT).size(), 
results.size());
+    List<ValidationOpResult> failed = results.stream().filter(result -> 
!result.isSuccess()).collect(Collectors.toList());
+    assertEquals(1, failed.size(), results.toString());
+    assertEquals(broken.getFileId(), failed.get(0).getOperation().getFileId());
+    assertTrue(failed.get(0).getException().isPresent());
+  }
+
+  /**
+   * Repair only validates the plan and reports the renames it would need; 
with the plan intact
+   * there is nothing to rename and the plan is left alone, whether or not 
this is a dry run.
+   */
+  @ParameterizedTest
+  @ValueSource(booleans = {true, false})
+  public void testSparkMainCompactRepair(boolean dryRun) throws Exception {
+    createPendingCompactions();
+    Set<String> fileIdsBefore = fileIdsOf(PENDING_COMPACTION_INSTANT);
+    String outputPath = outputPath("repair-" + dryRun);
+
+    SparkMain.doCompactRepair(jsc(), tablePath, PENDING_COMPACTION_INSTANT, 
outputPath, 2, dryRun);
+
+    assertTrue(readOperationResults(outputPath).isEmpty());
+    
assertTrue(pendingCompactionInstants().contains(PENDING_COMPACTION_INSTANT));
+    assertEquals(fileIdsBefore, fileIdsOf(PENDING_COMPACTION_INSTANT));
+  }
+
+  /**
+   * Unscheduling a plan takes the requested compaction instant off the 
timeline, unless this is a
+   * dry run. The other pending plans are left alone either way.
+   */
+  @ParameterizedTest
+  @CsvSource({"true, true", "true, false", "false, true", "false, false"})

Review Comment:
   `skipValidation` appears only in the signature and javadoc of 
`unscheduleCompactionPlan` and `unscheduleCompactionFileId` - neither body 
reads it - so two of these four rows repeat an identical run. Could this drop 
to `@ValueSource` over `dryRun` alone?



##########
hudi-cli/src/test/java/org/apache/hudi/cli/commands/TestTimelineCommand.java:
##########
@@ -0,0 +1,287 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.cli.commands;
+
+import org.apache.hudi.cli.HoodieCLI;
+import org.apache.hudi.cli.functional.CLIFunctionalTestHarness;
+import org.apache.hudi.cli.testutils.ShellEvaluationResultUtil;
+import org.apache.hudi.client.SparkRDDWriteClient;
+import org.apache.hudi.common.config.HoodieMetadataConfig;
+import org.apache.hudi.common.model.HoodieTableType;
+import org.apache.hudi.common.table.HoodieTableMetaClient;
+import org.apache.hudi.common.table.HoodieTableVersion;
+import org.apache.hudi.common.table.timeline.HoodieInstant;
+import org.apache.hudi.common.testutils.HoodieMetadataTestTable;
+import org.apache.hudi.common.testutils.HoodieTestTable;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.config.HoodieIndexConfig;
+import org.apache.hudi.config.HoodieWriteConfig;
+import org.apache.hudi.index.HoodieIndex;
+import org.apache.hudi.metadata.HoodieTableMetadata;
+import org.apache.hudi.metadata.HoodieTableMetadataWriter;
+import org.apache.hudi.metadata.SparkHoodieBackedTableMetadataWriter;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.shell.Shell;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static 
org.apache.hudi.common.testutils.HoodieTestDataGenerator.DEFAULT_FIRST_PARTITION_PATH;
+import static 
org.apache.hudi.common.testutils.HoodieTestDataGenerator.DEFAULT_SECOND_PARTITION_PATH;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Test cases for {@link TimelineCommand}.
+ */
+@Tag("functional")
+@SpringBootTest(properties = {"spring.shell.interactive.enabled=false", 
"spring.shell.command.script.enabled=false"})
+public class TestTimelineCommand extends CLIFunctionalTestHarness {
+
+  // Column offsets of the data table part of the rendered timeline, row 
number included.
+  private static final int COL_INSTANT = 1;
+  private static final int COL_ACTION = 2;
+  private static final int COL_STATE = 3;
+  private static final int COL_REQUESTED_TIME = 4;
+  private static final int COL_INFLIGHT_TIME = 5;
+  private static final int COL_COMPLETED_TIME = 6;
+  // Column offsets of the metadata table part, only rendered with 
--with-metadata-table.
+  private static final int COL_MT_ACTION = 7;
+  private static final int COL_MT_STATE = 8;
+
+  private static final String DATE_NO_SECONDS = "\\d{2}-\\d{2} \\d{2}:\\d{2}";
+  private static final String DATE_WITH_SECONDS = "\\d{2}-\\d{2} 
\\d{2}:\\d{2}:\\d{2}";
+
+  @Autowired
+  private Shell shell;
+
+  private String tablePath;
+  private HoodieTableMetaClient metaClient;
+  private String rollbackInstantTime;
+
+  /**
+   * Builds a table whose active timeline holds two completed commits, a 
completed rollback of a
+   * third commit and one commit left in the requested state, with the 
metadata table enabled so
+   * that the metadata table timeline is populated too.
+   */
+  @BeforeEach
+  public void init() throws Exception {
+    HoodieCLI.conf = storageConf();
+    String tableName = tableName();
+    tablePath = tablePath(tableName);
+
+    new TableCommand().createTable(
+        tablePath, tableName, HoodieTableType.COPY_ON_WRITE.name(),
+        "", HoodieTableVersion.current().versionCode(), 
"org.apache.hudi.common.model.HoodieAvroPayload");
+    metaClient = HoodieTableMetaClient.reload(HoodieCLI.getTableMetaClient());
+
+    Map<String, String> partitionAndFileId = new HashMap<>();
+    partitionAndFileId.put(DEFAULT_FIRST_PARTITION_PATH, "file-1");
+    partitionAndFileId.put(DEFAULT_SECOND_PARTITION_PATH, "file-2");
+
+    HoodieWriteConfig config = 
HoodieWriteConfig.newBuilder().withPath(tablePath)
+        .withMetadataConfig(
+            // Column Stats Index is disabled, since this table is built with 
empty commit metadata
+            
HoodieMetadataConfig.newBuilder().withMetadataIndexColumnStats(false).build())
+        .withRollbackUsingMarkers(false)
+        
.withIndexConfig(HoodieIndexConfig.newBuilder().withIndexType(HoodieIndex.IndexType.INMEMORY).build())
+        .build();
+
+    try (HoodieTableMetadataWriter metadataWriter = 
SparkHoodieBackedTableMetadataWriter.create(
+        metaClient.getStorageConf(), config, context)) {
+      HoodieTestTable testTable = HoodieMetadataTestTable.of(metaClient, 
metadataWriter, Option.of(context))
+          .withPartitionMetaFiles(DEFAULT_FIRST_PARTITION_PATH, 
DEFAULT_SECOND_PARTITION_PATH)
+          
.addCommit("100").withBaseFilesInPartitions(partitionAndFileId).getLeft()
+          
.addCommit("101").withBaseFilesInPartitions(partitionAndFileId).getLeft()
+          .addInflightCommit("102");
+      testTable.withBaseFilesInPartitions(partitionAndFileId);
+
+      try (SparkRDDWriteClient client = new SparkRDDWriteClient(context(), 
config)) {
+        client.rollback("102");
+      }
+      // left behind on the timeline so that the incomplete timeline is not 
empty
+      testTable.addRequestedCommit("103");
+    }
+
+    HoodieCLI.refreshTableMetadata();
+    metaClient = HoodieCLI.getTableMetaClient();
+    rollbackInstantTime = metaClient.getActiveTimeline().getRollbackTimeline()
+        .filterCompletedInstants().lastInstant().get().requestedTime();
+  }
+
+  @Test
+  public void testShowActive() {
+    Object result = shell.evaluate(() -> "timeline show active");
+    assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+
+    List<List<String>> rows = renderedRows(result.toString());
+    assertEquals(instantTimes(metaClient), rows.stream().map(r -> 
r.get(COL_INSTANT)).collect(Collectors.toSet()));
+    assertEquals(metaClient.getActiveTimeline().countInstants(), rows.size());
+
+    List<String> commit100 = rowOf(rows, "100");
+    assertEquals("commit", commit100.get(COL_ACTION));
+    assertEquals(HoodieInstant.State.COMPLETED.toString(), 
commit100.get(COL_STATE));
+    // a completed commit has all three instant files, so all three 
modification times are rendered
+    assertTrue(commit100.get(COL_REQUESTED_TIME).matches(DATE_NO_SECONDS), 
commit100.toString());
+    assertTrue(commit100.get(COL_INFLIGHT_TIME).matches(DATE_NO_SECONDS), 
commit100.toString());
+    assertTrue(commit100.get(COL_COMPLETED_TIME).matches(DATE_NO_SECONDS), 
commit100.toString());
+
+    List<String> commit103 = rowOf(rows, "103");
+    assertEquals("commit", commit103.get(COL_ACTION));
+    assertEquals(HoodieInstant.State.REQUESTED.toString(), 
commit103.get(COL_STATE));
+    // only the requested file exists for it, the other two states render as a 
dash
+    assertTrue(commit103.get(COL_REQUESTED_TIME).matches(DATE_NO_SECONDS), 
commit103.toString());
+    assertEquals("-", commit103.get(COL_INFLIGHT_TIME));
+    assertEquals("-", commit103.get(COL_COMPLETED_TIME));
+
+    assertEquals("rollback", rowOf(rows, rollbackInstantTime).get(COL_ACTION));
+  }
+
+  @Test
+  public void testShowActiveWithLimitAndSorting() {
+    Object result = shell.evaluate(() -> "timeline show active --limit 2 
--sortBy Instant --desc true");
+    assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+
+    List<List<String>> rows = renderedRows(result.toString());
+    assertEquals(2, rows.size());
+    List<String> allInstants = new ArrayList<>(instantTimes(metaClient));
+    allInstants.sort(String::compareTo);
+    // descending order on the instant time, cut to the first two rows
+    assertEquals(allInstants.get(allInstants.size() - 1), 
rows.get(0).get(COL_INSTANT));
+    assertEquals(allInstants.get(allInstants.size() - 2), 
rows.get(1).get(COL_INSTANT));
+  }
+
+  @Test
+  public void testShowActiveHeaderOnly() {
+    Object result = shell.evaluate(() -> "timeline show active --headeronly 
true");
+    assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+
+    assertTrue(renderedRows(result.toString()).isEmpty(), result.toString());
+    assertTrue(result.toString().contains("Instant"), result.toString());
+    assertTrue(result.toString().contains(EMPTY_TABLE_CELL), 
result.toString());
+  }
+
+  @Test
+  public void testShowActiveWithRollbackInfoAndSeconds() {
+    Object result = shell.evaluate(
+        () -> "timeline show active --show-rollback-info true 
--show-time-seconds true");
+    assertTrue(ShellEvaluationResultUtil.isSuccess(result));
+
+    List<List<String>> rows = renderedRows(result.toString());
+    // the rollback instant is annotated with the commit it rolls back
+    assertEquals("rollback Rolls back 102", rowOf(rows, 
rollbackInstantTime).get(COL_ACTION));
+    // instants that were not rolled back carry no annotation
+    assertEquals("commit", rowOf(rows, "100").get(COL_ACTION));

Review Comment:
   A completed rollback deletes its target instant, so nothing on this timeline 
is ever annotated `Rolled back by` and the requested-plan branch of 
`getInstantToRollback` never runs. Could the fixture also carry a pending 
rollback, so both arms of `getRollbackInfoString` are covered?



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

Reply via email to