You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "nastra (via GitHub)" <gi...@apache.org> on 2023/05/15 07:33:55 UTC
[GitHub] [iceberg] nastra commented on a diff in pull request #7597: Core: Remove deprecated AssertHelpers usage
nastra commented on code in PR #7597:
URL: https://github.com/apache/iceberg/pull/7597#discussion_r1193427971
##########
core/src/test/java/org/apache/iceberg/actions/TestBinPackStrategy.java:
##########
@@ -307,46 +307,51 @@ public void testNumOuputFiles() {
@Test
public void testInvalidOptions() {
- AssertHelpers.assertThrows(
- "Should not allow max size smaller than target",
- IllegalArgumentException.class,
- () -> {
- defaultBinPack()
- .options(ImmutableMap.of(BinPackStrategy.MAX_FILE_SIZE_BYTES, Long.toString(1 * MB)));
- });
-
- AssertHelpers.assertThrows(
- "Should not allow min size larger than target",
- IllegalArgumentException.class,
- () -> {
- defaultBinPack()
- .options(
- ImmutableMap.of(BinPackStrategy.MIN_FILE_SIZE_BYTES, Long.toString(1000 * MB)));
- });
-
- AssertHelpers.assertThrows(
- "Should not allow min input size smaller than 1",
- IllegalArgumentException.class,
- () -> {
- defaultBinPack()
- .options(ImmutableMap.of(BinPackStrategy.MIN_INPUT_FILES, Long.toString(-5)));
- });
-
- AssertHelpers.assertThrows(
- "Should not allow min deletes per file smaller than 1",
- IllegalArgumentException.class,
- () -> {
- defaultBinPack()
- .options(ImmutableMap.of(BinPackStrategy.DELETE_FILE_THRESHOLD, Long.toString(-5)));
- });
-
- AssertHelpers.assertThrows(
- "Should not allow negative target size",
- IllegalArgumentException.class,
- () -> {
- defaultBinPack()
- .options(ImmutableMap.of(RewriteDataFiles.TARGET_FILE_SIZE_BYTES, Long.toString(-5)));
- });
+ Assertions.assertThatThrownBy(
+ () ->
+ defaultBinPack()
+ .options(
+ ImmutableMap.of(
+ BinPackStrategy.MAX_FILE_SIZE_BYTES, Long.toString(1 * MB))))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageStartingWith(
+ "Cannot set min-file-size-bytes greater than or equal to max-file-size-bytes");
+
+ Assertions.assertThatThrownBy(
+ () ->
+ defaultBinPack()
+ .options(
+ ImmutableMap.of(
+ BinPackStrategy.MIN_FILE_SIZE_BYTES, Long.toString(1000 * MB))))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageStartingWith(
+ "Cannot set min-file-size-bytes greater than or equal to max-file-size-bytes");
+
+ Assertions.assertThatThrownBy(
+ () ->
+ defaultBinPack()
+ .options(ImmutableMap.of(BinPackStrategy.MIN_INPUT_FILES, Long.toString(-5))))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessageStartingWith(
+ "Cannot set min-input-files is less than 1. All values less than 1 have the same effect as 1");
Review Comment:
`Cannot set min-input-files is less than 1` is slightly misleading. Could you maybe open a separate PR to slightly improve the error messaging in `BinPackStrategy`?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org