On Wed, 23 Apr 2025 23:50:06 GMT, Manukumar V S <m...@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. Changes requested by aivanov (Reviewer). test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 1296: > 1294: > 1295: /** > 1296: * Forcibly pass the test. This should be used in semi-automatic > tests when Suggestion: * Forcibly pass the test. This method should be used in semi-automatic tests when I'm inclined to use imperative: _“Use this method in….”_ test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 1298: > 1296: * Forcibly pass the test. This should be used in semi-automatic > tests when > 1297: * the test determines that all the conditions for passing the test > are met. > 1298: * This should not be used in cases where a resource is unavailable > or a Suggestion: * <p> * This method should not be used in cases where a resource is unavailable or a Again imperative could better. test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 1310: > 1308: * PassFailJFrame.forceFail("TEST FAILED (output file - " + > output + ")"); > 1309: * } > 1310: * </code></pre> Shall we scrap the sample completely? It doesn't add clarity. However, this new sample illustrates the use of both `forcePass` and `forceFail`. A section in the javadoc for the class could be better fit for such kind of example. Linking to a real test which uses these methods could be beneficial, too. ------------- PR Review: https://git.openjdk.org/jdk/pull/24837#pullrequestreview-2791385720 PR Review Comment: https://git.openjdk.org/jdk/pull/24837#discussion_r2058557831 PR Review Comment: https://git.openjdk.org/jdk/pull/24837#discussion_r2058562286 PR Review Comment: https://git.openjdk.org/jdk/pull/24837#discussion_r2058573792
