On Wed, 16 Nov 2022 14:02:32 GMT, Weibing Xiao <d...@openjdk.org> wrote:

>> print warning message for java.io.tmpdir when it is set through the command 
>> line and the value is not good for creating file folder.
>
> Weibing Xiao has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   the change according to review comment

src/java.base/share/classes/java/lang/System.java line 2249:

> 2247:         // Emit a warning if java.io.tmpdir is set via the command line 
> to a directory that doesn't exist
> 2248:         if (SystemProps.checkIfWarningRequired()) {
> 2249:             System.err.println("WARNING: java.io.tmpdir location does 
> not exist");

Change "location" to "directory", reflecting the exact check being done.

src/java.base/share/classes/jdk/internal/util/SystemProps.java line 48:

> 46: 
> 47:     /**
> 48:      * check if warning for custom java.io.tmpdir is required

Capitialize and add a period at the end.

src/java.base/share/classes/jdk/internal/util/SystemProps.java line 52:

> 50:      * @return a boolean value
> 51:      */
> 52:     public static boolean checkIfWarningRequired() {

I would rename the method to `checkIoTmpdir`; it will be easier to see the 
purpose in the call in System.

src/java.base/share/classes/jdk/internal/util/SystemProps.java line 70:

> 68:         HashMap<String, String> props = raw.cmdProperties();
> 69: 
> 70:         customTmpdir = props.get("java.io.tmpdir");

Move the assignment to line 98:

`customTmpdir = putIfAbsent(props, "java.io.tmpdir", 
raw.propDefault(Raw._java_io_tmpdir_NDX));`

It will return null if the property is not already set and save a little bit.
If it is set, it will return the custom directory.

test/jdk/java/io/File/TempDirectoryNotExisting.java line 45:

> 43: 
> 44:         String userDir = System.getProperty("user.home");
> 45:         String timeStamp = System.currentTimeMillis() + "";

A human readable string might be useful.  For example,  
"2022-11-16T15:10:50.622334Z"
`java.time.Instant.now().toString()`

test/jdk/java/io/File/TempDirectoryNotExisting.java line 49:

> 47: 
> 48:         if (args.length != 0) {
> 49:             if (args[0].equals("io")) {

This code to cover the cases could be a bit cleaner and without duplication as:
``` 
    for (String arg : args) {
        if (arg.equals("io")) {
            ...
        } else if (arg.equals("nio")) { 
            ...
        } else { 
           throw Exception("unknown case: " + arg);
        }
    }

test/jdk/java/io/File/TempDirectoryNotExisting.java line 123:

> 121:                 .filter(line -> line.equalsIgnoreCase(ioWarningMsg))
> 122:                 .collect(Collectors.toList());
> 123:         if (list.size() != 1) throw new Exception("counter of messages 
> is not one, but " + list.size());

If there's an error, print the list of messages; it can be very informative to 
see the unexpected messages, including the case where the exitValue is wrong.

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

PR: https://git.openjdk.org/jdk/pull/11174

Reply via email to