RFR: 8354450: Using a File with a path containing a trailing space should fail when alternative data streams are disabled (win) [v2]

[Brian Burkhalter]

On Wed, 16 Apr 2025 09:21:41 GMT, Mikhail Yankelevich <myankelevich at openjdk.org> wrote:

>> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8354450: Add specific conditional for trailing space; adjust test
>
> test/jdk/java/io/File/WinTrailingSpace.java line 28:
> 
>> 26: 
>> 27: import org.junit.jupiter.api.Test;
>> 28: import static org.junit.jupiter.api.Assertions.*;
> 
> nitpick: wildcard import

I am not fond of wildcard imports either, but it is common practice for static imports like this. I will change it though.

> test/jdk/java/io/File/WinTrailingSpace.java line 61:
> 
>> 59:             f.delete();
>> 60:             f.createNewFile();
>> 61:             assertFalse(f.exists()); // should not reach here
> 
> Do you think this might be easier to debug if there was a message in there rather than an `assertFalse`? In case of an error this will just say failed. However in the failed case where the file wasn't actually created and the error wasn't triggered either it should pass in the current state. This will be more confusing when debugging, especially in case of an intermittent issue.
>  My feeling is that for the long run it might be better to have something like 
> ```java 
> assertFalse("File was created. File exists", f.exists());
> assertEquals("No error thrown", "Error Thrown"); 
> 
> 
> What do you think?

I agree that something like you suggest would be better. I'll take a closer look.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24635#discussion_r2047397807
PR Review Comment: https://git.openjdk.org/jdk/pull/24635#discussion_r2047399125