RFR: 8344942: Template-Based Testing Framework [v76]

Tue Jun 3 14:36:06 UTC 2025

On Tue, 3 Jun 2025 11:43:48 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> **Goal**
>> We want to generate Java source code:
>> - Make it easy to generate variants of tests. E.g. for each offset, for each operator, for each type, etc.
>> - Enable the generation of domain specific fuzzers (e.g. random expressions and statements).
>> 
>> Note: with the Template Library draft I was already able to find a [list of bugs](https://bugs.openjdk.org/issues/?jql=labels%20%3D%20template-framework%20ORDER%20BY%20created%20DESC%2C%20summary%20DESC).
>> 
>> **How to get started**
>> When reviewing, please start by looking at:
>> https://github.com/openjdk/jdk/blob/d21a8aabaf3b191e851b6997c11bb30fcd0f942f/test/hotspot/jtreg/testlibrary_tests/template_framework/examples/TestSimple.java#L60-L76
>> 
>> We have a Template with two arguments. They are typed (Integer and String). We then apply the arguments `template.withArgs(42, "7")`, producing a `TemplateWithArgs`. This can then be `render`ed to a String. And then that can be compiled and executed with the CompileFramework.
>> 
>> Second, look at this advanced test:
>> https://github.com/openjdk/jdk/blob/77079807042fc5a3af04e0ccccad4ecd89e21cdb/test/hotspot/jtreg/testlibrary_tests/template_framework/examples/TestAdvanced.java#L102-L119
>> 
>> And then for a "tutorial", look at:
>> `test/hotspot/jtreg/testlibrary_tests/template_framework/examples/TestTutorial.java`
>> 
>> It shows these features:
>> - The `body` of a Template is essentially a list of `Token`s that are concatenated.
>> - Templates can be nested: a `TemplateWithArgs` is also a `Token`.
>> - We can use `#name` replacements to directly format values into the String. If we had proper String Templates in Java, we would not need this feature.
>> - We can use `$var` to make variable names unique: if we applied the same template twice, we would get variable collisions. `$var` is then replaced with e.g. `var_7` in one template use and `var_42` in the other template use.
>> - The use of `Hook`s to insert code into outer (earlier) code locations. This is useful, for example, to insert fields on demand.
>> - The use of recursive templates, and `fuel` to limit the recursion.
>> - `Name`s: useful to register field and variable names in code scopes.
>> 
>> Next, look at the documentation in. This file is the heart of the Template Framework, and describes all the important features.
>> https://github.com/openjdk/jdk/blob/d21a8aabaf3b191e851b6997c11bb30fcd0f942f/test/hotspot/jtreg/compiler/lib/template_framework/Template.java#L31-L76
>> 
>> For a better experience, you may want...
>
> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   rename View -> FilteredSet

Thanks for the update! Almost there, some last comments and then we're good to go :-)

test/hotspot/jtreg/compiler/lib/template_framework/CodeFrame.java line 95:

> 93:      * where we would possibly want to make field or variable definitions during the insertion
> 94:      * that are not just local to the insertion but affect the {@link CodeFrame} that we
> 95:      * {@link Hook#anchor} earlier and are now {@link Hook#insert}ing into.

It complains that `addName` cannot be found. Suggestion to use `{@link Template#addDataName}/
     * {@link Template#addStructuralName}` instead:

Suggestion:

     * Creates a special frame, which has a {@link #parent} but uses the {@link NameSet}
     * from the parent frame, allowing {@link Template#addDataName}/
     * {@link Template#addStructuralName} to persist in the outer frame when the current frame 
     * is exited. This is necessary for {@link Hook#insert},  where we would possibly want to 
     * make field or variable definitions during the insertion that are not just local to the 
     * insertion but affect the {@link CodeFrame} that we {@link Hook#anchor} earlier and are 
     * now {@link Hook#insert}ing into.

test/hotspot/jtreg/compiler/lib/template_framework/CodeFrame.java line 114:

> 112:             throw new RuntimeException("Internal error: Duplicate Hook in CodeFrame: " + hook.name());
> 113:         }
> 114:         hookCodeLists.put(hook, new Code.CodeList(new ArrayList<Code>()));

Suggestion:

        hookCodeLists.put(hook, new Code.CodeList(new ArrayList<>()));

test/hotspot/jtreg/compiler/lib/template_framework/Hook.java line 26:

> 24: package compiler.lib.template_framework;
> 25: 
> 26: import java.util.List;

Unused:

Suggestion:

test/hotspot/jtreg/compiler/lib/template_framework/Name.java line 29:

> 27: import java.util.Map;
> 28: import java.util.ArrayList;
> 29: import java.util.List;

Unused:
Suggestion:

test/hotspot/jtreg/compiler/lib/template_framework/NameSet.java line 89:

> 87:             if (w < 0) {
> 88:                 throw new RuntimeException("Negative weight not allowed: " + w);
> 89:             }

I thought zero is also not allowed?

test/hotspot/jtreg/compiler/lib/template_framework/Renderer.java line 32:

> 30: import java.util.regex.Matcher;
> 31: import java.util.regex.Pattern;
> 32: import java.util.stream.Stream;

Some are unused:

Suggestion:

import java.util.List;
import java.util.regex.MatchResult;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

test/hotspot/jtreg/compiler/lib/template_framework/Renderer.java line 358:

> 356:             // If the character was not found, we want to have the rest of the
> 357:             // String s, so instead of "-1" take the end/length of the String.
> 358:             dollar  = (dollar == -1)  ? s.length() : dollar;

`s.length()` could be called once before the loop and then reused inside the loop.

test/hotspot/jtreg/compiler/lib/template_framework/Renderer.java line 384:

> 382: 
> 383:     /**
> 384:      * We are parsing a part now. Befor the part, there was either a "#" or "$":

Suggestion:

     * We are parsing a part now. Before the part, there was either a "#" or "$":

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

PR Review: https://git.openjdk.org/jdk/pull/24217#pullrequestreview-2892355999
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2123827132
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2123829199
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2123830659
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2123806572
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2123937058
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2123820068
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2124044533
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2124038113