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

[Emanuel Peter]

On Fri, 16 May 2025 07:03:12 GMT, Manuel Hässig <mhaessig at openjdk.org> wrote:

>> Hmm, maybe I have to use more words to be more explicit. I replaced the two lines with this:
>> 
>>    45     /**
>> ~  46      * There can be at most one Renderer instance at any time.
>> ~  47      *
>> +  48      * When using nested templates, the user of the Template Framework may be tempted to first render
>> +  49      * the nested template to a {@link String}, and then use this {@link String} as a token in an outer
>> +  50      * {@link Template#body}. This would be a bad pattern: the outer and nested {@link Template} would
>> +  51      * be rendered separately, and could not interact. For example, the nested {@link Template} would
>> +  52      * not have access to the scopes of the outer {@link Template}. The inner {@link Template} could
>> +  53      * not access {@link Name}s and {@link Hook}s from the outer {@link Template}. The user might assume
>> +  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      *
>> +  57      * Instead, the user should create a {@link TemplateToken} from the inner {@link Template}, and
>> +  58      * use that {@link TemplateToken} in the {@link Template#body} of the outer {@link Template}.
>> +  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      *
>> +  62      * The {@link Renderer} instance exists during the whole rendering process. Should the user ever
>> +  63      * attempt to render a nested {@link Template} to a {@link String}, we would detect that there is
>> +  64      * already a {@link Renderer} instance for the outer {@link Template}, and throw a {@link RendererException}.                                                                                                                          
>>    65      */
>
> That provides much more clarity. However, the amount of text makes me think part of it should probably go into the Javadoc of the `Renderer` class to explain its usage further.

I don't have a super strong opinion here.

The whole class is package private, so the user will never see this anyway. It is more of a note to the reader of the internal code.

I though it makes most sense to put it with the static `renderer` field, to give a justification why we have it.

Alternatively, I could also make the `RendererException` message more verbose. But I don't want to burden the user with all that justification text, I just want to give the user an alternative way to get done what the user wants to get done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24217#discussion_r2092501713