RFR: 8183021: Fix failing jshell tests on Windows

[Jan Lahoda]

On 28.6.2017 17:42, Jonathan Gibbons wrote:
>
>
> 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?

A version where the test framework can be configured to accept both \r\n 
and \n as line endings:
http://cr.openjdk.java.net/~jlahoda/8183021/webrev.03/

Jan

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