rdblue commented on code in PR #9131:
URL: https://github.com/apache/iceberg/pull/9131#discussion_r1414416904
##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/sql/TestSelect.java:
##########
@@ -419,6 +420,12 @@ public void
testInvalidTimeTravelBasedOnBothAsOfAndTableIdentifier() {
})
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot do time-travel based on both table identifier and
AS OF");
+
+ // using branch_b1 in the table identifier and VERSION AS OF
+ Assertions.assertThatThrownBy(
+ () -> sql("SELECT * FROM %s.branch_b1 VERSION AS OF %s",
tableName, snapshotId))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Cannot do time-travel based on both table identifier and
AS OF");
Review Comment:
I suspect this should also be a separate test case.
In general, I'm _very_ skeptical of PRs that add new code and assertions to
existing test cases. We want test cases to be narrowly focused. When in doubt,
add a new test method.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
