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

Tue Feb 6 14:29:58 UTC 2024

On Tue, 6 Feb 2024 11:14:15 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> **Bigger goal**
>> 
>> I am tired of always writing IR tests where I have to write elaborate code in the `@Run` method to create inputs, and then run the test method to create `gold` output values in (hopefully) interpreter mode, and then later verify the results of the compiled method.
>> 
>> Hence, I now introduce the "Setup" method, which can create custom argument values.
>> In a later RFE, I will implement automatic result verification, which implicitly intercepts the inputs and outputs of the test method, and compares the behaviour of the interpreter and the compiled code. That way, the pattern will be:
>> 
>> 
>> @Setup
>> ... specify your arguments and fields ...
>> 
>> -> intercept arguments, cache them
>> 
>> @Test
>> ... write a test with arbitrary inputs and outputs...
>> 
>> -> interpreter mode: intercept outputs and cache them (i.e. gold values)
>> -> compiled mode: intercept outputs and compare them to the gold values.
>> 
>> (optional)
>> @Check
>> .. do custom verification...
>> 
>> 
>> **In this RFE: the Setup Method**
>> 
>> The first step is the setup method.
>> 
>> Example:
>> https://github.com/openjdk/jdk/blob/08670a5d45b1c45e954e2dc85eb8c92e11e48fb2/test/hotspot/jtreg/testlibrary_tests/ir_framework/examples/SetupExample.java#L49-L87
>
> Emanuel Peter has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - fix whitespace from last commit
>  - Apply suggestions from code review
>    
>    Co-authored-by: Christian Hagedorn <christian.hagedorn at oracle.com>

Some comments about the README.

test/hotspot/jtreg/compiler/lib/ir_framework/README.md line 44:

> 42: Arguments can be provided with `@Arguments(values = {...})` by providing well-specified inputs for each individual argument. Alternatively, a setup method can be chosen with `@Arguments(setup = "setupMethodName")`, which computes arguments and can also set fields.
> 43: 
> 44: More information on normal test methods with a precise definition can be found in the Javadocs of [Test](./Test.java). Concrete examples on how to specify a normal test can be found in [BaseTestsExample](../../../testlibrary_tests/ir_framework/examples/BaseTestExample.java).

Since we now change the README and the definition of two kinds of tests, I also suggest to rename `BaseTestExample.java` to `NormalTestExample.java`.

test/hotspot/jtreg/compiler/lib/ir_framework/README.md line 47:

> 45: 
> 46: ##### Setup Method
> 47: A `@Setup` annotated method can provide custom arguments and set fields before a normal test is run. A normal test method can be annotated with `@Arguments(setup = "setupMethodName")` to specify which setup method is to be used.

Suggestion:

A `@Setup` annotated method can provide custom arguments and set fields before a normal test is run. A `@Test` annotated method can additionally be annotated with `@Arguments(setup = "setupMethodName")` to define the dedicated `@Setup` method.

test/hotspot/jtreg/compiler/lib/ir_framework/README.md line 49:

> 47: A `@Setup` annotated method can provide custom arguments and set fields before a normal test is run. A normal test method can be annotated with `@Arguments(setup = "setupMethodName")` to specify which setup method is to be used.
> 48: 
> 49: More information on checked tests with a precise definition can be found in the Javadocs of [Setup](./Setup.java). Concrete examples on how to specify a setup method can be found in [SetupExample](../../../testlibrary_tests/ir_framework/examples/SetupExample.java).

Suggestion:

More information on normal tests with `@Setup` methods together with a precise definition can be found in the Javadocs of [Setup](./Setup.java). Concrete examples on how to specify a setup method can be found in [SetupExample](../../../testlibrary_tests/ir_framework/examples/SetupExample.java).

test/hotspot/jtreg/compiler/lib/ir_framework/README.md line 52:

> 50: 
> 51: ##### Check Method
> 52: A `@Check` annotated method is invoked directly after a corresponding normal `@Test` annotated method. The user can perform various checks, such as test method return value and field value verification.

Suggestion:

A `@Check(test = "checkMethodName")` annotated method is invoked directly after the `@Test` annotated method `checkMethodName()` is executed. The user can perform various checks, such as test method return value and field value verification.

test/hotspot/jtreg/compiler/lib/ir_framework/README.md line 54:

> 52: A `@Check` annotated method is invoked directly after a corresponding normal `@Test` annotated method. The user can perform various checks, such as test method return value and field value verification.
> 53: 
> 54: More information on check methods with a precise definition can be found in the Javadocs of [Check](./Check.java). Concrete examples on how to specify check methods can be found in [CheckedTestsExample](../../../testlibrary_tests/ir_framework/examples/CheckedTestExample.java).

Maybe add here an additional paragraph:

`@Setup` and `@Check` methods can only be specified for normal but not for custom run tests (see next section).

test/hotspot/jtreg/compiler/lib/ir_framework/README.md line 57:

> 55: 
> 56: #### Custom Run Tests
> 57: The normal test method is invoked directly by the framework, and hence the user does not have control over the invokation.

This was probably copy pasted here by mistake since it's a description of a normal test.
Suggestion:

test/hotspot/jtreg/compiler/lib/ir_framework/README.md line 58:

> 56: #### Custom Run Tests
> 57: The normal test method is invoked directly by the framework, and hence the user does not have control over the invokation.
> 58: A custom run test gives full control over the invocation of the `@Test` annotated method to the user. The framework calls a dedicated `@Run` annotated method from which the user can invoke the `@Test` method according to their needs.

I suggest to add some more details what can be done and should be done (i.e. setup and verification):

Suggestion:

A custom run test gives full control over the invocation of the `@Test` annotated method to the user which includes argument and field setup as well as result and field value verification. The framework calls a dedicated `@Run` annotated method from which the user can invoke the `@Test` method according to their needs.

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

PR Review: https://git.openjdk.org/jdk/pull/17557#pullrequestreview-1865316377
PR Review Comment: https://git.openjdk.org/jdk/pull/17557#discussion_r1479880187
PR Review Comment: https://git.openjdk.org/jdk/pull/17557#discussion_r1479881026
PR Review Comment: https://git.openjdk.org/jdk/pull/17557#discussion_r1479886584
PR Review Comment: https://git.openjdk.org/jdk/pull/17557#discussion_r1479902278
PR Review Comment: https://git.openjdk.org/jdk/pull/17557#discussion_r1479904846
PR Review Comment: https://git.openjdk.org/jdk/pull/17557#discussion_r1479915969
PR Review Comment: https://git.openjdk.org/jdk/pull/17557#discussion_r1479918249