RFR(S): 8204654: [testbug] Fix pattern matching in jstat tests.
Thomas Stüfe
thomas.stuefe at gmail.com
Thu Jun 14 12:54:30 UTC 2018
On Thu, Jun 14, 2018 at 2:48 PM, Lindenmaier, Goetz
<goetz.lindenmaier at sap.com> wrote:
> Hi Thomas,
>
> thanks for your review!
> New webrev:
> http://cr.openjdk.java.net/~goetz/wr18/8204654-fixJstatTest/02/
> I'll run this through our testing again and push tomorrow if all
> is green.
>
>> consider better comment:
>> -# The awk scripts parsing the output do not respect any locale
>> dependent setting.
>> +# The awk scripts parsing the jstat output expect it to be in en-us locale.
> Fixed.
>
>> I assume
>> ([0-9])|-+
>> should match a single whole number for column "FindClass", or "-"?
> Yes, fixed.
>
>> similar here: + misplaced:
>> - ([0-9]+\.[0-9])|-+
>> + ([0-9]+\.[0-9]+)|-
> Also fixed. Great catch!
>
>> Also, it would be helpful if you could print column name or at least
>> number atop the matching sequences, in a comment, that makes the
>> reading easer.
> Yes, but there are a row of similar files I don't touch. Don't want to do
> this for all of them.
>
I totally understand :)
>> But this is hard to read! Any chance of splitting this expression into
>> sub expressions?
> Yes, this is ugly.
> But I think the whole tests should be rewritten to do the parsing in Java,
> as the jstatd tests do. I think this is out of scope here.
Sure.
>
> Best regards,
> Goetz.
I do not need to see a new webrev.
..Thomas
>
>
>
>>
>>
>> Rest seems fine.
>>
>> ..Thomas
>>
>> On Mon, Jun 11, 2018 at 3:14 PM, Lindenmaier, Goetz
>> <goetz.lindenmaier at sap.com> wrote:
>> > Hi,
>> >
>> > please review this test fix:
>> > http://cr.openjdk.java.net/~goetz/wr18/8204654-fixJstatTest/01
>> >
>> > gcCauseOutput1.awk:
>> > The pattern scans 11 numbers, while the output contains 13. Also, more '-'
>> are possible then checked.
>> >
>> > The other awk scripts need to check more patterns where '-' can appear.
>> >
>> > We have a machine with user.country=de where jstat prints ',' instead of '.'
>> in numbers. Explicitly
>> > start with user.country=en as already done for user.language=en.
>> >
>> > I also refactored the common flags to a variable in utils.sh ... so there won't
>> be the need
>> > to edit all these files once more :)
>> >
>> > Best regards,
>> > Goetz
>> >
>> > (Sorry for the second mail, first missed the bug text ...)
More information about the serviceability-dev
mailing list