RFR: 8355441: Remove antipattern from PassFailJFrame.forcePass javadoc [v2]

Alexey Ivanov aivanov at openjdk.org
Thu Apr 24 16:59:47 UTC 2025


On Thu, 24 Apr 2025 15:06:48 GMT, Manukumar V S <mvs at openjdk.org> wrote:

>> The javadoc for PassFailJFrame.forcePass suggests an anti-pattern of forcibly passing the test if a resource is unavailable.
>> 
>> If a resource is unavailable or a feature isn't supported, the test should throw jtreg.SkippedException.
>> 
>> PassFailJFrame.forcePass should be used in semi-automatic tests when the test determines that all the conditions for passing the test are met.
>> Please refer: JDK-8355366 and https://github.com/openjdk/jdk/pull/24820
>> 
>> Testing
>> This is a javadoc change, so not testing required.
>
> Manukumar V S has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comments fixed : Formatting changes, added reference to a real test

Do you mind expanding the documentation for `forcePass` and `forceFail` by adding a new section to `PassFailJFrame` description? I submitted [JDK-8355515](https://bugs.openjdk.org/browse/JDK-8355515).

test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 1305:

> 1303:      * <p> A sample usage can be found in this test :
> 1304:      * <a href="https://github.com/openjdk/jdk/blob/master/test/jdk/java/awt/FileDialog/SaveFileNameOverrideTest.java#L84">SaveFileNameOverrideTest.java</a>
> 1305:      *

Suggestion:

     * Forcibly pass the test.
     * <p>
     * Use this method in semi-automatic tests when
     * the test determines that all the conditions for passing the test are met.
     * <p>
     * <strong>Do not use</strong> this method in cases where a resource is unavailable or a
     * feature isn't supported, throw {@code jtreg.SkippedException} instead.
     *
     * <p>A sample usage can be found in
     * <a href="https://github.com/openjdk/jdk/blob/7283c8b/test/jdk/java/awt/FileDialog/SaveFileNameOverrideTest.java#L84">{@code
     * SaveFileNameOverrideTest.java}</a>

This way looks better to me.

You want a permanent link; the test could change in the future.

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

Changes requested by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24837#pullrequestreview-2791839556
PR Review Comment: https://git.openjdk.org/jdk/pull/24837#discussion_r2058836519


More information about the client-libs-dev mailing list