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