RFR: 8183021: Fix failing jshell tests on Windows

[Jan Lahoda]

On 28.6.2017 01:12, Robert Field wrote:
>
> On 06/27/17 15:05, Jan Lahoda wrote:
>> On 27.6.2017 20:29, Robert Field wrote:
>>> Looks fine.
>>
>> Sorry, I was too eager and sent this before the tests were finished.
>> Turned out even the snippet.source() may contain "\n". Given the use
>> of "\n" seems mostly benign (except for these UI tests), and trying to
>> change "\n" to "\r\n" for the snippet.source() seems less safe, I
>> tried to fix this solely in the test. Using "[\n]" will translate to
>> "[\r\n]" inside the test framework on Windows, which will match the
>> sole \n.
>
> Right, that will work in this case.  However, were it actually a \r\n it
> would fail without being [\n]+ -- but that all seems twisty-turny and
> only addresses this one case.
>
> How about, instead of fixing this one test
> (ToolMultilineSnippetHistoryTest), instead change line 123 of UITesting
> from:
>
>          expected = expected.replaceAll("\n",
> System.getProperty("line.separator"));
>
> To:
>
>          expected = expected.replaceAll("\n", "\\R");
>
> ?
>
> Where (from Pattern) --
>
> Linebreak matcher
> \R     Any Unicode linebreak sequence, is equivalent to
> \u000D\u000A|[\u000A\u000B\u000C\u000D\u0085\u2028\u2029]

I was thinking of generalizing the \n handling, but I think I prefer the 
framework and tests to be more strict - at least until we have more 
tests that want to be more lax. (Not sure if we could use \R, as the 
test framework should allow to check a difference between '\r' and '\n', 
I think, as changing '\n' to '\r' could have visible effects. To be more 
lax, we might be able to use \r?\n, so something alike.)

But a good pointer for \R - maybe this test could be made lax using \R:
http://cr.openjdk.java.net/~jlahoda/8183021/webrev.02/

What do you think?

Thanks,
    Jan

>
> -Robert
>
>>
>> Updated webrev:
>> http://cr.openjdk.java.net/~jlahoda/8183021/webrev.01/
>>
>> Sorry for trouble.
>>
>>>
>>> We seem to have a general issue of not using platform line ending, I've
>>
>> I've noticed that, but except for these tests seems mostly benign, right?
>>
>> Jan
>>
>>> created:
>>>
>>>      https://bugs.openjdk.java.net/browse/JDK-8183022
>>>
>>> -Robert
>>>
>>> On 06/27/17 10:49, Jan Lahoda wrote:
>>>> Hi,
>>>>
>>>> Two JShell tests are failing on Win:
>>>> -jdk/jshell/ToolBasicTest.java, seems that
>>>> System.getProperty("user.home") may return forward slashes, but the
>>>> tests expects the canonical backward slash. The proposed solution is
>>>> to normalize the path using Paths.get(...).
>>>> -jdk/jshell/ToolMultilineSnippetHistoryTest.java, seems JShellTool
>>>> prints \n, while the test expects \r\n. The proposed solution is to
>>>> change the relevant place in JShellTool to use println to improve
>>>> testability.
>>>>
>>>> Bug:
>>>> https://bugs.openjdk.java.net/browse/JDK-8183021
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~jlahoda/8183021/webrev.00/
>>>>
>>>> Thanks,
>>>>     Jan
>>>
>