RFR 8202690: jdk/jshell/ToolBasicTest.java failed in testOpenFileOverHttp() and testOpenLocalFileUrl()

[Jan Lahoda]

Looks OK to me.

Jan

On 7.5.2018 17:35, Robert Field wrote:
> Thanks Chris!
>
> For those that like to see webrevs, here is one for Chris' change:
>
>       http://cr.openjdk.java.net/~rfield/8202690v0.webrev/
>
> -Robert
>
>
>
> On 05/07/18 02:25, Chris Yin wrote:
>> Please have a review for below suggested fix for 8202690 to handle
>> windows platform correctly after 8199912, thanks
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8202690
>>
>> Suggested fix as below:
>>
>> diff -r caf05d64138f
>> src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java
>> ---
>> a/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.javaMon
>> May 07 07:44:49 2018 +0530
>> +++
>> b/src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.javaMon
>> May 07 17:12:20 2018 +0800
>> @@ -42,6 +42,7 @@
>>  import java.lang.module.ModuleReference;
>>  import java.net.MalformedURLException;
>>  import java.net.URI;
>> +import java.net.URISyntaxException;
>>  import java.net.URL;
>>  import java.nio.charset.Charset;
>>  import java.nio.file.FileSystems;
>> @@ -3000,19 +3001,34 @@
>>                      regenerateOnDeath = false;
>>                      scanner = new Scanner(cmdin);
>>                  } else {
>> -                    Path path = toPathResolvingUserHome(filename);
>> +                    Path path = null;
>> +                    URL url = null;
>>                      String resource;
>> -                    if (Files.exists(path)) {
>> +                    try {
>> +                        path = toPathResolvingUserHome(filename);
>> +                    } catch (InvalidPathException ipe) {
>> +                        try {
>> +                            url = new URL(filename);
>> +                            if
>> (url.getProtocol().equalsIgnoreCase("file")) {
>> +                                path = Paths.get(url.toURI());
>> +                            }
>> +                        } catch (MalformedURLException |
>> URISyntaxException e) {
>> +                            throw new FileNotFoundException(filename);
>> +                        }
>> +                    }
>> +                    if (path != null && Files.exists(path)) {
>>                          scanner = new Scanner(new
>> FileReader(path.toString()));
>>                      } else if ((resource = getResource(filename)) !=
>> null) {
>>                          scanner = new Scanner(new
>> StringReader(resource));
>>                      } else {
>> -                        try {
>> -                            var url = new URL(filename);
>> -                            scanner = new Scanner(url.openStream());
>> -                        } catch (MalformedURLException mue) {
>> -                            throw new FileNotFoundException(filename);
>> +                        if (url == null) {
>> +                            try {
>> +                                url = new URL(filename);
>> +                            } catch (MalformedURLException mue) {
>> +                                throw new
>> FileNotFoundException(filename);
>> +                            }
>>                          }
>> +                        scanner = new Scanner(url.openStream());
>>                      }
>>                  }
>>                  try (var scannerIOContext = new
>> ScannerIOContext(scanner)) {
>> diff -r caf05d64138f test/langtools/jdk/jshell/ToolBasicTest.java
>> --- a/test/langtools/jdk/jshell/ToolBasicTest.javaMon May 07 07:44:49
>> 2018 +0530
>> +++ b/test/langtools/jdk/jshell/ToolBasicTest.javaMon May 07 17:12:20
>> 2018 +0800
>> @@ -501,7 +501,7 @@
>>          compiler.writeToFile(path, "int a = 10;int b = 20;int c = a +
>> b;\n");
>>          for (String s : new String[]{"/o", "/open"}) {
>>              test(
>> -                    (a) -> assertCommand(a, s + " file://" +
>> path.toString(), ""),
>> +                    (a) -> assertCommand(a, s + " " + path.toUri(), ""),
>>                      (a) -> assertCommand(a, "a", "a ==> 10"),
>>                      (a) -> assertCommand(a, "b", "b ==> 20"),
>>                      (a) -> assertCommand(a, "c", "c ==> 30")
>>
>>
>> Regards,
>> Chris
>