RFR: 8355444: [java.io] Use @requires tag instead of exiting based on "os.name" property value [v2]
Brian Burkhalter
bpb at openjdk.org
Fri Apr 25 16:55:56 UTC 2025
On Fri, 25 Apr 2025 16:33:59 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:
>> test/jdk/java/io/File/MacPathTest.java line 28:
>>
>>> 26: * @summary Tests paths on macOS
>>> 27: * @requires (os.family == "mac")
>>> 28: */
>>
>> The missing `@test` on this looked odd, since that would mean that this test wasn't being run at all so far.
>>
>> I went back and looked at the original RFR which introduced this test for https://bugs.openjdk.org/browse/JDK-7130915. The RFR is here https://mail.openjdk.org/pipermail/core-libs-dev/2012-June/010621.html. Going through the webrevs posted there, it appears that this was initially a shell test and had `@test` declaration.
>>
>> Then in https://bugs.openjdk.org/browse/JDK-8181912 we refactored it to be a java jtreg test. The RFR for that is here https://mail.openjdk.org/pipermail/core-libs-dev/2017-June/048225.html. Going through this refactor RFR, the final webrev that was settled upon and integrated appears to be this https://mail.openjdk.org/pipermail/core-libs-dev/2017-June/048319.html. So what that refactoring did was (https://cr.openjdk.org/~mli/8181912/webrev.01/) it moved the `@test` declaration to a new file `test/java/io/File/MacPath.java` which is what then launches this `test/jdk/java/io/File/MacPathTest.java`'s main method using `ProcessBuilder`. So this `MacPathTest.java` isn't really the jtreg `@test`.
>>
>> Given this, i think we shouldn't be adding this `@test` declaration here and `MacPath.java` already has the necessary `@requires (os.family == "mac")`.
>>
>> What we should probably do (if you prefer in a different issue/PR), is perhaps add a comment to this file that it gets launched through `MacPath.java` and also maybe remove the `os.name` checks in the `main()` method of this class.
>
> Thanks, @jaikiran, for the detailed investigation. I went partway down that road but clearly not far enough. I'll update it.
I replaced the `@test` block with a plain comment in 5d24645 and removed the vestigial check of `os.name`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24860#discussion_r2060577848
More information about the core-libs-dev
mailing list