On Wed, 28 Oct 2020 14:30:30 GMT, Chris Hegarty <[email protected]> wrote:
>> I've added some negative tests that test for the different failure
>> conditions.
>
> Thanks for adding additional test coverage @JornVernee.
>
> Writing a tight implementation of assertThrows is non-trivial - I'm not sure
> that the version you have will fail if the exception is not thrown? Either
> way maybe we can reuse some of the junit machinery for this purpose, e.g.:
>
> static final Class<IllegalArgumentException> IAE =
> IllegalArgumentException.class;
>
> public void testReorderTypeMismatch() throws Throwable {
> MethodHandle mh = MethodHandles.empty(MethodType.methodType(void.class,
> int.class, int.class, String.class));
> MethodType newType = MethodType.methodType(void.class, double.class,
> String.class);
> var exception = expectThrows(IAE, () ->
> MethodHandles.permuteArguments(mh, newType, 0, 0, 1));
> assertMatches(exception.getMessage(), ".*parameter types do not match
> after reorder.*");
> }
>
> private static void assertMatches(String str, String pattern) {
> if (!str.matches(pattern)) {
> throw new AssertionError("'" + str + "' did not match the pattern '"
> + pattern + "'.");
> }
> }
My last comment, that suggested to use expectThrows was not appropriate for
this junit test ( I mistakenly assumed that that functional was available, but
it is not ) - withdrawn.
Thanks for adding the extra check in assertThrows. LGTM.
-------------
PR: https://git.openjdk.java.net/jdk/pull/878
