RFR: 8344942: Template-Based Testing Framework [v32]
Christian Hagedorn
chagedorn at openjdk.org
Fri May 16 12:05:14 UTC 2025
On Fri, 16 May 2025 10:10:44 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:
>
> fix up review suggestions
Some more comments, I will take this up again on Monday. Looking good so far :-) Great work!
test/hotspot/jtreg/compiler/lib/template_framework/CodeFrame.java line 36:
> 34: * {@link Hook}s can be added to a frame, which allows code to be inserted at that location later.
> 35: * When a {@link Hook} is {@link Hook#set}, this separates the Template into an outer and inner
> 36: * {@link CodeFrame}, ensuring that names that are {@link Template#defineName}'d inside the inner frame
`define` and `defineName` seem to no longer exist.
test/hotspot/jtreg/compiler/lib/template_framework/CodeFrame.java line 91:
> 89:
> 90: /**
> 91: * Creates a special frame, which has a {@link parent} but uses the {@link NameSet}
Suggestion:
* Creates a base frame, which has no {@link #parent}.
*/
public static CodeFrame makeBase() {
return new CodeFrame(null, false);
}
/**
* Creates a normal frame, which has a {@link #parent} and which defines an inner
* {@link NameSet}, for the names that are generated inside this frame. Once this
* frame is exited, the name from inside this frame are not available anymore.
*/
public static CodeFrame make(CodeFrame parent) {
return new CodeFrame(parent, false);
}
/**
* Creates a special frame, which has a {@link #parent} but uses the {@link NameSet}
test/hotspot/jtreg/compiler/lib/template_framework/Renderer.java line 39:
> 37: *
> 38: * The {@link Renderer} instance keeps track of the current frames,
> 39: * see {@link TemplateFrame} and {@link CodeFrame}.
I suggest to mention that we render a tokenized template with this class. You can also use Javadocs `@see`:
Suggestion:
* The {@link Renderer} class renders a tokenized {@link Template} in the form of a {@link TemplateToken}.
* It also keeps track of the states during a nested Template rendering. There can only be a single
* {@link Renderer} active at any point, since there are static methods that reference
* {@link Renderer#getCurrent}.
*
* <p>
* The {@link Renderer} instance keeps track of the current frames.
*
* @see TemplateFrame
* @see CodeFrame
test/hotspot/jtreg/compiler/lib/template_framework/Renderer.java line 47:
> 45: /**
> 46: * There can be at most one Renderer instance at any time.
> 47: *
Suggestion:
*
* <p>
test/hotspot/jtreg/compiler/lib/template_framework/Renderer.java line 56:
> 54: * that the inner {@link Template} has access to the outer {@link Template}, but they would actually
> 55: * be separated. This could lead to unexpected behavior or even bugs.
> 56: *
Suggestion:
*
* <p>
test/hotspot/jtreg/compiler/lib/template_framework/Renderer.java line 61:
> 59: * This way, the inner and outer {@link Template}s get rendered together, and the inner {@link Template}
> 60: * has access to the {@link Name}s and {@link Hook}s of the outer {@link Template}.
> 61: *
Suggestion:
*
* <p>
test/hotspot/jtreg/compiler/lib/template_framework/Renderer.java line 71:
> 69: private TemplateFrame baseTemplateFrame;
> 70: private TemplateFrame currentTemplateFrame;
> 71: private CodeFrame baseCodeFrame;
Can be made final:
Suggestion:
private final TemplateFrame baseTemplateFrame;
private TemplateFrame currentTemplateFrame;
private final CodeFrame baseCodeFrame;
test/hotspot/jtreg/compiler/lib/template_framework/TemplateFrame.java line 31:
> 29: /**
> 30: * The {@link TemplateFrame} is the frame for a {@link Template}, i.e. the corresponding
> 31: * {@link TemplateToken}. It ensures that each template use has its own unique {@link id}
Suggestion:
* {@link TemplateToken}. It ensures that each template use has its own unique {@link #id}
test/hotspot/jtreg/compiler/lib/template_framework/TemplateFrame.java line 35:
> 33: * replacements, which combine the key-value pairs from the template argument and the
> 34: * {@link Template#let} definitions. The {@link parent} relationship provides a trace
> 35: * for the use chain of templates. The {@link fuel} is reduced over this chain, to give
Suggestion:
* {@link Template#let} definitions. The {@link #parent} relationship provides a trace
* for the use chain of templates. The {@link #fuel} is reduced over this chain, to give
test/hotspot/jtreg/compiler/lib/template_framework/TemplateFrame.java line 47:
> 45: final Map<String, String> hashtagReplacements = new HashMap<>();
> 46: final float fuel;
> 47: float fuelCost;
Some can be made `private`. Maybe you want to double check if you don't want to make them all private and just offer accessor methods for those that are used by the `Renderer` instead:
Suggestion:
final TemplateFrame parent;
private final int id;
private final Map<String, String> hashtagReplacements = new HashMap<>();
final float fuel;
private float fuelCost;
test/hotspot/jtreg/compiler/lib/template_framework/TemplateFrame.java line 73:
> 71: return;
> 72: }
> 73: throw new RendererException("Duplicate hashtag replacement for #" + key);
Could be simplified to:
Suggestion:
if (hashtagReplacements.putIfAbsent(key, value) != null) {
throw new RendererException("Duplicate hashtag replacement for #" + key);
}
test/hotspot/jtreg/compiler/lib/template_framework/TemplateToken.java line 28:
> 26: /**
> 27: * Represents a Template with filled arguments, ready for instantiation, either
> 28: * as a {@link Token} inside another {@link Template} or with {@link #render}.
Now that we renamed `fill()` to `asToken()`, how about:
Suggestion:
* Represents a tokenized {@link Template} (after calling {@code asToken()}) ready for
* instantiation either as a {@link Token} inside another {@link Template} or as
* a {@link String} with {@link #render}.
test/hotspot/jtreg/compiler/lib/template_framework/TemplateToken.java line 41:
> 39: * Represents a zero-argument {@link TemplateToken}, already filled with arguments, ready for
> 40: * instantiation either as a {@link Token} inside another {@link Template} or
> 41: * with {@link #render}.
Following the above suggestion, how about:
Suggestion:
* Represents a tokenized zero-argument {@link Template} ready for instantiation
* either as a {@link Token} inside another {@link Template} or as a {@link String}
* with {@link #render}.
test/hotspot/jtreg/compiler/lib/template_framework/TemplateToken.java line 61:
> 59: /**
> 60: * Represents a one-argument {@link TemplateToken}, already filled with arguments, ready for instantiation
> 61: * either as a {@link Token} inside another {@link Template} or with {@link #render}.
Similarly:
Suggestion:
* Represents a tokenized one-argument {@link Template}, already filled with arguments, ready for
* instantiation either as a {@link Token} inside another {@link Template} or as a {@link String}
* with {@link #render}.
test/hotspot/jtreg/compiler/lib/template_framework/TemplateToken.java line 87:
> 85: /**
> 86: * Represents a two-argument {@link TemplateToken}, already filled with arguments, ready for instantiation
> 87: * either as a {@link Token} inside another {@link Template} or with {@link #render}.
Suggestion:
* Represents a tokenized two-argument {@link Template}, already filled with arguments, ready for
* instantiation either as a {@link Token} inside another {@link Template} or as a {@link String}
* with {@link #render}.
test/hotspot/jtreg/compiler/lib/template_framework/TemplateToken.java line 117:
> 115: /**
> 116: * Represents a three-argument {@link TemplateToken}, already filled with arguments, ready for instantiation
> 117: * either as a {@link Token} inside another {@link Template} or with {@link #render}.
Suggestion:
* Represents a tokenized three-argument {@link TemplateToken}, already filled with arguments, ready for
* instantiation either as a {@link Token} inside another {@link Template} or as a {@link String}
* with {@link #render}.
test/hotspot/jtreg/compiler/lib/template_framework/Token.java line 33:
> 31: * The {@link Template#body} and {@link Hook#set} are given a list of tokens, which are either
> 32: * {@link Token}s or {@link String}s or some permitted boxed primitives. These are then parsed
> 33: * and all non {@link Token}s are converted to {@link StringToken}s. The parsing also flattens
Suggestion:
* and all non-{@link Token}s are converted to {@link StringToken}s. The parsing also flattens
test/hotspot/jtreg/compiler/lib/template_framework/Token.java line 51:
> 49: throw new IllegalArgumentException("Unexpected tokens: null");
> 50: }
> 51: List<Token> outputList = new ArrayList<Token>();
Can be simplified:
Suggestion:
List<Token> outputList = new ArrayList<>();
-------------
PR Review: https://git.openjdk.org/jdk/pull/24217#pullrequestreview-2846308913
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092913603
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092914802
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092871329
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092871616
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092871775
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092871953
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092874930
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092876424
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092877304
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092881685
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092898464
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092854996
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092857864
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092859476
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092861819
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092862742
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092844398
PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092845249
More information about the hotspot-compiler-dev
mailing list