rdblue commented on code in PR #5234:
URL: https://github.com/apache/iceberg/pull/5234#discussion_r953201851
##########
core/src/test/java/org/apache/iceberg/TestRowDelta.java:
##########
@@ -1414,18 +1415,69 @@ public void testRowDeltaCaseSensitivity() {
}
@Test
- public void testRowDeltaToBranchUnsupported() {
- AssertHelpers.assertThrows(
- "Should reject committing row delta to branch",
- UnsupportedOperationException.class,
- "Cannot commit to branch someBranch: org.apache.iceberg.BaseRowDelta
does not support branch commits",
- () ->
+ public void testBranchConflictingDeletes() {
Review Comment:
I think that this class either needs to copy all of the tests and validate
against a branch (like `testValidateDataFilesExistDefaults`), or be
parameterized. I prefer parameterized.
If the tests are parameterized for `main` and `some_branch`, you could
refactor all of the `commit` calls to something like this:
```
public void commitOperation(SnapshotProducer producer) {
if (branch.equals("main")) {
producer.commit();
} else {
producer.toBranch(branch).commit();
}
}
```
##########
core/src/test/java/org/apache/iceberg/TestRowDelta.java:
##########
@@ -1414,18 +1415,69 @@ public void testRowDeltaCaseSensitivity() {
}
@Test
- public void testRowDeltaToBranchUnsupported() {
- AssertHelpers.assertThrows(
- "Should reject committing row delta to branch",
- UnsupportedOperationException.class,
- "Cannot commit to branch someBranch: org.apache.iceberg.BaseRowDelta
does not support branch commits",
- () ->
+ public void testBranchConflictingDeletes() {
Review Comment:
I think that this class either needs to copy all of the tests and validate
against a branch (like `testValidateDataFilesExistDefaults`), or be
parameterized. I prefer parameterized.
If the tests are parameterized for `main` and `some_branch`, you could
refactor all of the `commit` calls to something like this:
```java
public void commitOperation(SnapshotProducer producer) {
if (branch.equals("main")) {
producer.commit();
} else {
producer.toBranch(branch).commit();
}
}
```
--
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]
