On Tue, 30 Sep 2025 19:11:08 GMT, Alexey Ivanov <[email protected]> wrote:
>> test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 1846:
>>
>>> 1844: InvocationTargetException {
>>> 1845: try {
>>> 1846: validate();
>>
>> Do you know when `build` is invoked? I see this `validate` is removed in
>> favor of having `builder.validate()` on line 508. Overall, the changes look
>> like they're cleanly refactored but this is the biggest change I see,
>> although minor.
>
> I don't understand your question… Yet I'll try to answer it.
>
> `Builder.build()` is invoked as the last step of using the `Builder` class to
> configure and create an object of `PassFailJFrame`. The usual sequence is:
>
>
> PassFailJFrame.builder()
> .instructions(INSTRUCTIONS)
> .testUI(SampleManualTest::createTestUI)
> .build()
> .awaitAndCheck();
>
>
> `Builder.build()` was the only method that called `PassFailJFrame(Builder)`
> constructor. Before the calling the constructor, `validate` was used to
> validate the parameters in `Builder` and throw an exception if the required
> parameters, such as instructions, aren't provided.
>
> Now, there are two places where `PassFailJFrame(Builder)` is called, the new
> one is at lines 489–493. An instance of `Builder` is created there and is
> passed to the `PassFailJFrame(Builder)` constructor without calling
> `validate()`. Since `validate` doesn't return a value (and it shouldn't), the
> `validate` method cannot be called in a chained sequence.
>
> No other statements are allowed in constructors before calling `this`,
> therefore there's no way to create a `Builder` instance and call `validate`
> on it before passing it to the `PassFailJFrame` constructor. (Yes, I know
> that other statements are allowed in JDK 26 (and 25?), but previous versions
> would have to come up with another way.)
>
> Validating the instance of `Builder` inside the `PassFailJFrame` constructor
> covers both cases, thus it ensures `Builder.validate` is always called before
> creating UI.
Understood, that answered my question. Thanks for the details on this.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27321#discussion_r2392832301
