RFR: 8358697: java/awt/font/TextLayout/MyanmarTextTest.java gives false positive result when preconditions not met [v5]
Alexey Ivanov
aivanov at openjdk.org
Thu Jun 19 10:06:55 UTC 2025
On Thu, 19 Jun 2025 03:42:21 GMT, Manukumar V S <mvs at openjdk.org> wrote:
>> Issue:
>> MyanmarTextTest.java produces a false positive result when some of the test preconditions are not met. It checks whether certain fonts are present in the system, for example, 'Padauk' on Linux. If the required font is missing, the test simply returns early, and the test ends up passing, which is incorrect. Ideally, it should throw a jtreg.SkippedException when the necessary preconditions are not satisfied.
>>
>> Another scenario is that the test passes on headless machines even though it creates GUI components. Ideally, when GUI components are created in code running on a headless machine, a HeadlessException should be thrown. However, since MyanmarTextTest.java exits before reaching the point where the GUI is created (due to unmet preconditions), it incorrectly reports a pass. This behavior may lead to a misinterpretation of the test as being headless, which it is not.
>>
>> Fix:
>> Need to throw jtreg.SkippedException in cases where some pre-conditions for running the test are not met.
>>
>> Testing:
>> Tested using mach5 in all available platforms and got full PASS.
>
> Manukumar V S has updated the pull request incrementally with one additional commit since the last revision:
>
> Combined the fonts in to one list, formatting changes
Changes requested by aivanov (Reviewer).
test/jdk/java/awt/font/TextLayout/MyanmarTextTest.java line 56:
> 54: private static final String TEXT = "\u1000\u103C";
> 55:
> 56: private static final String OS_NAME = System.getProperty("os.name").toLowerCase();
The OS name isn't needed any longer.
test/jdk/java/awt/font/TextLayout/MyanmarTextTest.java line 63:
> 61: "Myanmar Text",
> 62: "Noto Sans Myanmar");
> 63: private static final String FONT_NAME = selectFontName();
Suggestion:
"Noto Sans Myanmar");
private static final String FONT_NAME = selectFontName();
A blank line.
test/jdk/java/awt/font/TextLayout/MyanmarTextTest.java line 76:
> 74: System.err.println("Checked fonts: " + fontList);
> 75: throw new SkippedException("Required fonts not installed for OS: "
> 76: + OS_NAME + ". Checked fonts: " + fontList);
Suggestion:
throw new SkippedException("No suitable font found out of the list: "
+ String.join(", ", FONT_CANDIDATES));
I see no value in printing the same information twice, the exception message is *always* printed, and it contains all the required details.
test/jdk/java/awt/font/TextLayout/MyanmarTextTest.java line 145:
> 143: .filter(FONT_CANDIDATES::contains)
> 144: .findFirst()
> 145: .orElse(null);
I'm for preserving the original wrapping style where `.` align:
Suggestion:
return Arrays.stream(GraphicsEnvironment
.getLocalGraphicsEnvironment()
.getAvailableFontFamilyNames())
.filter(FONT_CANDIDATES::contains)
.findFirst()
.orElse(null);
-------------
PR Review: https://git.openjdk.org/jdk/pull/25879#pullrequestreview-2942345012
PR Review Comment: https://git.openjdk.org/jdk/pull/25879#discussion_r2156613372
PR Review Comment: https://git.openjdk.org/jdk/pull/25879#discussion_r2156614008
PR Review Comment: https://git.openjdk.org/jdk/pull/25879#discussion_r2156624016
PR Review Comment: https://git.openjdk.org/jdk/pull/25879#discussion_r2156627677
More information about the client-libs-dev
mailing list