RFR: 8183021: Fix failing jshell tests on Windows

[Jonathan Gibbons]

On 6/28/17 4:38 AM, Jan Lahoda wrote:
> 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

Tests that are designed to test the line-end handling should be strict, 
but tests for other functionality but which incidentally have line 
endings in their output can be more lax. Can the framework be configurable?

Good tip on \R, by the way. I know other places where that could be used.

-- Jon
>
>>
>> -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
>>>>
>>