On Wed, 28 Oct 2020 14:30:30 GMT, Chris Hegarty <che...@openjdk.org> 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