Introduce from and to attributes on @EnumSource (#4185) #4221
Conversation
| @@ -63,6 +63,8 @@ | |||
| * first parameter of the {@code @ParameterizedTest} method is used. | |||
| * | |||
| * @see #names | |||
| * @see #from | |||
| * @see #to | |||
There was a problem hiding this comment.
maybe also add it to the mode() method javadoc
| @@ -164,6 +172,15 @@ void testWithEnumSourceRegex(ChronoUnit unit) { | |||
| } | |||
| // end::EnumSource_regex_example[] | |||
|
|
|||
| // tag::EnumSource_range_exclude_example[] | |||
| @ParameterizedTest | |||
| @EnumSource(mode = EXCLUDE, from = "HOURS", to = "DAYS", names = { "HALF_DAYS" }) | |||
There was a problem hiding this comment.
this test exemple will depend on the JDK. So, if the order is change, it can impact your test. is it not better to use a home made enum for the test exemple ?
There was a problem hiding this comment.
Changing the order of the enum in the JDK would be a breaking change which is not usually done so I think we're safe here.
There was a problem hiding this comment.
I think this is slightly more readable:
| @EnumSource(mode = EXCLUDE, from = "HOURS", to = "DAYS", names = { "HALF_DAYS" }) | |
| @EnumSource(from = "HOURS", to = "DAYS", mode = EXCLUDE, names = { "HALF_DAYS" }) |
| @EnumSource(from = "HOURS", to = "DAYS") | ||
| void testWithEnumSourceRange(ChronoUnit unit) { | ||
| assertTrue(EnumSet.of(ChronoUnit.HOURS, ChronoUnit.HALF_DAYS, ChronoUnit.DAYS).contains(unit)); | ||
| } |
| } | ||
| E from = enumSource.from().isEmpty() ? constants[0] : Enum.valueOf(enumClass, enumSource.from()); | ||
| E to = enumSource.to().isEmpty() ? constants[constants.length - 1] : Enum.valueOf(enumClass, enumSource.to()); | ||
| return EnumSet.range(from, to); |
There was a problem hiding this comment.
What happen if from > to or if from == to ?
It maybe need to be specify somewhere in the doc, and have a test on this cases
There was a problem hiding this comment.
I think we should allow from == to (like EnumSet.range) but throw an error in case from > to with a good message.
@yhkuo41 Could you please add that?
| @EnumSource(from = "HOURS", to = "DAYS") | ||
| void testWithEnumSourceRange(ChronoUnit unit) { | ||
| assertTrue(EnumSet.of(ChronoUnit.HOURS, ChronoUnit.HALF_DAYS, ChronoUnit.DAYS).contains(unit)); | ||
| } |
| * @see #mode | ||
| */ | ||
| String to() default ""; | ||
|
|
There was a problem hiding this comment.
Please add @since 5.12 to the Javadoc and @API(status = EXPERIMENTAL, since = "5.12") to the declaration for both attributes.
| @@ -164,6 +172,15 @@ void testWithEnumSourceRegex(ChronoUnit unit) { | |||
| } | |||
| // end::EnumSource_regex_example[] | |||
|
|
|||
| // tag::EnumSource_range_exclude_example[] | |||
| @ParameterizedTest | |||
| @EnumSource(mode = EXCLUDE, from = "HOURS", to = "DAYS", names = { "HALF_DAYS" }) | |||
There was a problem hiding this comment.
I think this is slightly more readable:
| @EnumSource(mode = EXCLUDE, from = "HOURS", to = "DAYS", names = { "HALF_DAYS" }) | |
| @EnumSource(from = "HOURS", to = "DAYS", mode = EXCLUDE, names = { "HALF_DAYS" }) |
| return EnumSet.allOf(enumClass); | ||
| E[] constants = enumClass.getEnumConstants(); | ||
| if (constants.length == 0) { | ||
| return EnumSet.noneOf(enumClass); |
There was a problem hiding this comment.
We should validate that both from and to are empty in this case, shouldn't we?
Ensure `from` and `to` are empty when the enum has no constants, and validate `from` comes before `to` like `EnumSet.range`. Mark the `from` and `to` attributes as EXPERIMENTAL. Resolves junit-team#4185.
Resolves #4185.
Overview
I hereby agree to the terms of the JUnit Contributor License Agreement.
Definition of Done
@APIannotations