amogh-jahagirdar commented on code in PR #4071:
URL: https://github.com/apache/iceberg/pull/4071#discussion_r848925451


##########
core/src/test/java/org/apache/iceberg/TestSnapshotManager.java:
##########
@@ -250,4 +248,305 @@ public void testCherryPickOverwrite() {
         lastSnapshotId, table.currentSnapshot().snapshotId());
     validateTableFiles(table, FILE_A, FILE_B);
   }
+
+  @Test
+  public void testCreateBranch() {
+    table.newAppend()
+        .appendFile(FILE_A)
+        .commit();
+    long snapshotId = table.currentSnapshot().snapshotId();
+    // Test a basic case of creating a branch
+    table.manageSnapshots()
+        .createBranch("branch1", snapshotId)
+        .commit();
+    SnapshotRef expectedBranch = table.ops().refresh().ref("branch1");
+    Assert.assertTrue(expectedBranch != null &&
+            
expectedBranch.equals(SnapshotRef.branchBuilder(snapshotId).build()));
+
+    // Trying to create a branch with an existing name should fail
+    AssertHelpers.assertThrows("Should fail validation check if we try and 
create branch again",
+        IllegalArgumentException.class, "Ref branch1 already exists",
+        () -> table.manageSnapshots().createBranch("branch1", 
snapshotId).commit());
+
+    // Trying to create another branch within the same chain
+    AssertHelpers.assertThrows("Should fail validation check if we try and 
create branch again",
+        IllegalArgumentException.class, "Ref branch2 already exists",
+        () -> table.manageSnapshots().createBranch("branch2", 
snapshotId).createBranch("branch2", snapshotId).commit());
+  }
+
+
+  @Test
+  public void testCreateTag() {
+    table.newAppend()
+        .appendFile(FILE_A)
+        .commit();
+    long snapshotId = table.currentSnapshot().snapshotId();
+    // Test a basic case of creating a tag
+    table.manageSnapshots()
+        .createTag("tag1", snapshotId)
+        .commit();
+    SnapshotRef expectedTag = table.ops().refresh().ref("tag1");
+
+    Assert.assertTrue(expectedTag != null &&
+        expectedTag.equals(SnapshotRef.tagBuilder(snapshotId).build()));
+
+    // Trying to create a tag with an existing name should fail
+    AssertHelpers.assertThrows("Should fail validation check if we try and 
create tag again",
+        IllegalArgumentException.class, "Ref tag1 already exists",
+        () -> table.manageSnapshots().createTag("tag1", snapshotId).commit());
+
+    // Trying to create another tag within the same chain
+    AssertHelpers.assertThrows("Should fail validation check if we try and 
create tag again",
+        IllegalArgumentException.class, "Ref tag2 already exists",
+        () -> table.manageSnapshots()
+            .createBranch("tag2", snapshotId)
+            .createBranch("tag2", snapshotId).commit());
+  }
+
+  @Test
+  public void testRemoveBranch() {
+    table.newAppend()
+        .appendFile(FILE_A)
+        .commit();
+    long snapshotId = table.currentSnapshot().snapshotId();
+    // Test a basic case of creating and then removing a branch and tag
+    table.manageSnapshots()
+        .createBranch("branch1", snapshotId)
+        .commit();
+    table.manageSnapshots()
+        .removeBranch("branch1")
+        .commit();
+
+    TableMetadata updated = table.ops().refresh();
+    SnapshotRef expectedBranch = updated.ref("branch1");
+    Assert.assertTrue(expectedBranch == null);
+
+    // Test chained creating and removal of branch and tag
+    table.manageSnapshots()
+        .createBranch("branch2", 1)
+        .removeBranch("branch2")
+        .commit();
+    updated = table.ops().refresh();
+    Assert.assertTrue(updated.ref("branch2") == null);
+
+    AssertHelpers.assertThrows("Should fail validation check if we try to 
remove a non-existant branch",
+        IllegalArgumentException.class, "No such branch non-existing",
+        () -> table.manageSnapshots().removeBranch("non-existing").commit());
+
+    AssertHelpers.assertThrows("Should fail validation check if we try to 
remove main",
+        IllegalArgumentException.class, "Cannot remove main branch",
+        () -> 
table.manageSnapshots().removeBranch(SnapshotRef.MAIN_BRANCH).commit());
+  }
+
+  @Test
+  public void testRemoveTag() {
+    table.newAppend()
+        .appendFile(FILE_A)
+        .commit();
+    long snapshotId = table.currentSnapshot().snapshotId();
+    // Test a basic case of creating and then removing a branch and tag
+    table.manageSnapshots()
+        .createTag("tag1", snapshotId)
+        .commit();
+    table.manageSnapshots()
+        .removeTag("tag1")
+        .commit();
+    TableMetadata updated = table.ops().refresh();
+    SnapshotRef expectedTag = updated.ref("tag1");
+    Assert.assertTrue(expectedTag == null);
+
+    // Test chained creating and removal of a tag
+    table.manageSnapshots()
+        .createTag("tag2", 1)
+        .removeTag("tag2")

Review Comment:
   Yeah I think we do this in UpdateProperties as well. I wasn't sure if there 
would be any other future APIs we would want to include in SnapshotManager 
where for some reason this chaining would be supported so I leaned towards 
being less restrictive although I really can't think of any case where this is 
super valuable. I'll leave it as is for now 



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to