On Mon, 2 Jun 2025 17:30:20 GMT, Chen Liang <li...@openjdk.org> wrote:
>> Hannes Greule has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> don't return null but latest as fallback
>
> test/langtools/tools/javap/ClassFileVersionTest.java line 85:
>
>> 83: @ParameterizedTest
>> 84: @MethodSource("classFiles")
>> 85: void test(byte[] classFile, boolean shouldError) throws Throwable {
>
> I recommend adding another String argument indicating the case name -
> otherwise, if junit fails, the error message reports the argument to string,
> which is not informative on which class file exactly is being tested.
>
> An alternative approach can be inlining the arguments to `createClassFile` to
> the case arguments, like
>
> void test(boolean shouldError, int major, int minor, AccessFlag... classFlags)
>
> And junit provides a more useful error message for such arguments.
Good idea, I adjusted the test method signature. Seems like junit doesn't like
the varargs, but that's okay.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25569#discussion_r2122046019
