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 > <[email protected]> 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 ...)
