API: Remove deprecated AssertHelpers usage #7468
Conversation
|
@akshayakp97 thanks for looking into this. It seems there's an overlap with #7482, so you might want to sync up with @liuxiaocs7. |
|
@nastra thanks for looking at my PR! I will update my PR to exclude the overlapping files from |
api/src/test/java/org/apache/iceberg/TestPartitionSpecValidation.java
Outdated
Show resolved
Hide resolved
nastra
changed the title
api/src/test/java/org/apache/iceberg/expressions/TestEvaluator.java
Outdated
Show resolved
Hide resolved
api/src/test/java/org/apache/iceberg/expressions/TestExpressionHelpers.java
Outdated
Show resolved
Hide resolved
| () -> Iterables.getLast(concat5)); | ||
|
|
||
| Assertions.assertThatThrownBy(() -> Iterables.getLast(concat5)) | ||
| .isInstanceOf(NoSuchElementException.class); |
There was a problem hiding this comment.
would be good to check for the specific error msg here
There was a problem hiding this comment.
thanks for the review. I don't see any error message in this case.
Refer - https://github.com/google/guava/blob/master/guava/src/com/google/common/collect/Iterables.java#L849
There was a problem hiding this comment.
ah ok, thanks for checking. Alternatively we could use .hasMessage(null) to indicate that there's no message I guess
api/src/test/java/org/apache/iceberg/transforms/TestBucketing.java
Outdated
Show resolved
Hide resolved
| Assertions.assertThatThrownBy( | ||
| () -> PartitionSpec.builderFor(SCHEMA).year("ts").year("ts").build()) | ||
| .hasMessageContaining("Cannot use partition name more than once") | ||
| .isInstanceOf(IllegalArgumentException.class); |
There was a problem hiding this comment.
nit: in other places typically the .hasMessage.. part comes after .isInstanceOf(), so we should probably make this consistent here as well
There was a problem hiding this comment.
just one minor comment around consistency, but other than that, this LGTM. @jackye1995 do you have any further comments?
Summary
apimodule