RFR(S): 8204654: [testbug] Fix pattern matching in jstat tests.

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Thu Jun 14 12:48:55 UTC 2018


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.

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

Best regards,
 Goetz.



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