RFR: 8358697: java/awt/font/TextLayout/MyanmarTextTest.java gives false positive result when preconditions not met [v4]
Alexey Ivanov
aivanov at openjdk.org
Wed Jun 18 20:07:29 UTC 2025
On Wed, 18 Jun 2025 19:18:06 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:
>
> Added blank line before OS_NAME
Changes requested by aivanov (Reviewer).
test/jdk/java/awt/font/TextLayout/MyanmarTextTest.java line 59:
> 57: private static final String[] FONTS_WINDOWS = {"Myanmar Text", "Noto Sans Myanmar"};
> 58: private static final String[] FONTS_LINUX = {"Padauk", "Noto Sans Myanmar", "Myanmar Text"};
> 59: private static final String[] FONTS_MACOS = {"Myanmar MN", "Noto Sans Myanmar", "Myanmar Text"};
I'm unsure if Noto Sans fonts are installed by default on Windows or macOS.
Similarly, "Myanmar Text" is unlikely to be installed on Linux.
Since there's a list of possible fonts, the list could well be *platform-independent* — just look for a first match.
test/jdk/java/awt/font/TextLayout/MyanmarTextTest.java line 61:
> 59: private static final String[] FONTS_MACOS = {"Myanmar MN", "Noto Sans Myanmar", "Myanmar Text"};
> 60:
> 61: private static final String[] FONT_CANDIDATES = selectFontCandidates();
Suggestion:
private static final List<String> FONT_CANDIDATES =
List.of("Myanmar MN",
"Padauk",
"Myanmar Text",
"Noto Sans Myanmar");
test/jdk/java/awt/font/TextLayout/MyanmarTextTest.java line 74:
> 72: throw new SkippedException("Unsupported OS: " + OS_NAME);
> 73: }
> 74: if (FONT_NAME == null) {
With the changes that I propose, `FONT_NAME = selectFontName()`, this will be the only condition: if the font name is `null`, no suitable font is found — skip the test.
test/jdk/java/awt/font/TextLayout/MyanmarTextTest.java line 105:
> 103: main.setLayout(new BoxLayout(main, BoxLayout.Y_AXIS));
> 104: main.add(myanmarTF);
> 105: main.setBorder(BorderFactory.createEmptyBorder(7, 7, 7, 7));
I see no reason for removing this blank line.
test/jdk/java/awt/font/TextLayout/MyanmarTextTest.java line 153:
> 151: }
> 152:
> 153: private static String selectAvailableFont() {
private static String selectFontName() {
return Arrays.stream(GraphicsEnvironment
.getLocalGraphicsEnvironment()
.getAvailableFontFamilyNames())
.filter(FONT_CANDIDATES::contains)
.findFirst()
.orElse(null);
}
-------------
PR Review: https://git.openjdk.org/jdk/pull/25879#pullrequestreview-2940458706
PR Review Comment: https://git.openjdk.org/jdk/pull/25879#discussion_r2155365237
PR Review Comment: https://git.openjdk.org/jdk/pull/25879#discussion_r2155390963
PR Review Comment: https://git.openjdk.org/jdk/pull/25879#discussion_r2155402010
PR Review Comment: https://git.openjdk.org/jdk/pull/25879#discussion_r2155366028
PR Review Comment: https://git.openjdk.org/jdk/pull/25879#discussion_r2155392324
More information about the client-libs-dev
mailing list