RFR: 8355475: UNCTest should use an existing UNC path [v2]

Eirik Bjørsnøs eirbjo at openjdk.org
Thu Apr 24 17:42:46 UTC 2025


On Thu, 24 Apr 2025 16:12:17 GMT, Mikhail Yankelevich <myankelevich at openjdk.org> wrote:

>> Eirik Bjørsnøs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>> 
>>  - Cleanups suggested in review
>>  - Merge branch 'master' into unc-test-existing-path
>>  - UNCTest should use an existing UNC path
>
> test/jdk/java/net/URLConnection/UNCTest.java line 65:
> 
>> 63:      * @param hostName the name of the local computer
>> 64:      */
>> 65:     private static void skipIfAdministrativeSharesDisabled(String hostName) {
> 
> Minor: Is this necessary in the separate method? Seems to me to be a bit too much to move a single if statement. Do you think changing the main method to this might be a bit easier to read? 
> 
> 
> ...
>         // Get the "computer name" for this host
>         String hostName = InetAddress.getLocalHost().getHostName();
> 
>         // Skip the test if Administrative Shares is disabled
>         if (! new File("\\\" + hostName + "\\C$\\Windows").exists()) {
>             throw new SkippedException("Administrative shares not enabled");
>         }
> 
>         // Should always exist with Administrative Shares enabled
> ....
> 
> 
> It is fine as it is too if you prefer it this way 😃

Thanks for your suggestions @myankelev. I think I've adopted all of them, so rather than commenting on each, can you take another look at the last state and see if I maybe missed something?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24842#discussion_r2058938472


More information about the net-dev mailing list