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

[Mikhail Yankelevich]

On Wed, 16 Apr 2025 00:42:22 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> In `java.io.WinNTFileSystem::isInvalid`, replace an insufficient test for file path validity with a sufficient test for file path invalidity. Also, add a new test.
>
> 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

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?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24635#discussion_r2046493405
PR Review Comment: https://git.openjdk.org/jdk/pull/24635#discussion_r2046509227