RFR: 8324641: [IR Framework] Add Setup method to provide custom arguments and set fields [v5]

[Emanuel Peter]

On Mon, 5 Feb 2024 12:35:11 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   made ArgumentsProvider an interface
>
> test/hotspot/jtreg/compiler/lib/ir_framework/Arguments.java line 48:
> 
>> 46:      * Get the setup method name.
>> 47:      */
>> 48:     String[] setup() default {};
> 
> Should be a unique setup method. So, you can use `String` instead of `String[]`.
> Suggestion:
> 
>     String setup() default {};

done

> test/hotspot/jtreg/compiler/lib/ir_framework/test/TestVM.java line 762:
> 
>> 760:                          "Cannot use @Test " + testMethod + " for more than one @Run/@Check method. Found: "
>> 761:                          + m + ", " + attachedMethod);
>> 762:         TestFormat.check(test.hasDefaultArgumentsProvider(),
> 
> `hasDefaultArgumentsProvider()` -> `argumentsProvider.isDefault()` is only used here and uses an `instanceof` check in the super class with a sub class. This does not seem right. I suggest to do this entire format check over the `setupTestMap` instead:
> 
> Suggestion:
> 
>         TestFormat.check(!setupMethodMap.containsKey(testMethod.getName()),
> 
> This would also allow us to convert `ArgumentsProvider` into an interface (if we move `getArgumentsProvider()` out or to a separate `ArgumentsProviderBuilder` class).

done

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17557#discussion_r1478344812
PR Review Comment: https://git.openjdk.org/jdk/pull/17557#discussion_r1478346254