[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: JDK-8209604: [TEST] rewrite com/sun/jdi shell tests to java version - step2
From:       Alex Menkov <alexey.menkov () oracle ! com>
Date:       2018-08-31 1:03:39
Message-ID: 3aa14cbc-4375-f3f5-7ae3-61749ba0c40a () oracle ! com
[Download RAW message or body]

Agreed.
It looks like the root cause of the extra newlines.

--alex

On 08/29/2018 18:55, Chris Plummer wrote:
> I just stumbled across the following in Jdb.java. I think it might be 
> the source of the extra newlines on windows:
> 
>         public static final String lineSeparator = 
> System.getProperty("line.separator");
> 
>                         if (!jdbCommand.endsWith(lineSeparator)) {
>                                 logCmd = jdbCommand;
>                                 jdbCommand += lineSeparator;
>                         } else {
>                                 // we don't want to log the line separator
>                                 logCmd = jdbCommand.substring(0, jdbCommand.length() - 1);
>                         }
> 
> If this is the cause, I think the fix might be to just replace "1" with 
> lineSeparator.length().
> 
> Chris
> 
> On 8/27/18 4:36 PM, Chris Plummer wrote:
>> Yeah, I think the issue I had with extra newlines was with nsk tests. 
>> But it was something you only saw if the test failed, forcing the log 
>> to include all the output.
>>
>> thanks,
>>
>> Chris
>>
>> On 8/27/18 4:14 PM, Alex Menkov wrote:
>>> Hi Chris,
>>>
>>> This "newline stripping" logic was copied from nsk classes, where jdb 
>>> commands are represented as string constants and some of the 
>>> constants have newline at the end (for the commands which do not have 
>>> arguments), and others don't have.
>>> JdbCommand class in com/sun/jdi/lib/jdb/ has static factory methods 
>>> to create commands and it does not expect any new lines in the 
>>> commands, so this stripping logic is not required.
>>>
>>> I rechecked - jdb output looks good (no extra newlines).
>>> Maybe you mean some issues with nsk classes?
>>>
>>> --alex
>>>
>>> On 08/27/2018 14:55, Chris Plummer wrote:
>>>> Hi Alex,
>>>>
>>>> I noticed you no longer strip the trailing newline from JdbCommand. 
>>>> Are you sure when we print the command we are seeing the proper 
>>>> output of newlines (no extra ones). For example, we have the 
>>>> following in Jdb.java:
>>>>
>>>>                  System.out.println("> " + cmd.cmd);
>>>>
>>>>                  inputWriter.println(cmd.cmd);
>>>>
>>>> This looks like it will print an extra newline in cases where it was 
>>>> not stirpped. And I should point out that on windows we see a lot of 
>>>> extra newlines in the jdb output. I thought I had tracked this down 
>>>> once, but can't find the email were I explained the cause. But your 
>>>> change leads me to think it might have been faulty stripping of the 
>>>> newline in JdbCommand, and now you are making it so it never strips.
>>>>
>>>> thanks,
>>>>
>>>> Chris
>>>>
>>>> On 8/16/18 2:13 PM, Alex Menkov wrote:
>>>>> Hi all,
>>>>>
>>>>> Please review next chunk of shell->java test conversion.
>>>>> jira: https://bugs.openjdk.java.net/browse/JDK-8209604
>>>>> webrev: http://cr.openjdk.java.net/~amenkov/sh2java/step2/webrev/
>>>>>
>>>>> The fix contains some changes in library classes:
>>>>> Jdb.java - timeouts are updated (as per Dan note in one of previous 
>>>>> review, timeouts should respect timeout factor, Utils.adjustTimeout 
>>>>> implements the functionality);
>>>>> JdbCommand.java - new jdb commands (required by tests) are added.
>>>>>
>>>>> --alex
>>>>
>>>>
>>>>
>>
>>
> 
> 
[prev in list] [next in list] [prev in thread] [next in thread]