[PATCH] JMC-5499 Add XFlagChanged events to JVM Information or subpage

[Ken Dobson]

There's a ticket here about how we should handle empty tables
https://bugs.openjdk.java.net/browse/JMC-4009
I think it's an issue that might have a larger scope than just this bug but
it's definitely worth addressing.

Ken Dobson

On Thu, Jun 20, 2019 at 1:30 PM Alex Macdonald <almacdon at redhat.com> wrote:

> On Thu, Jun 20, 2019 at 9:05 AM Jessye Coleman Shapiro <
> jescolem at redhat.com>
> wrote:
>
> > Hi Alex,
> >
> > I will make those changes and send an updated patch.
> >
>
> Err, whoops. I was reviewing your JMC-5706 patch at the same time as this
> one, and must have applied the other patch when I thought it was this one.
> The previous review actually applies to your JMC-5706 patch, so I'll copy
> the review on that thread.
>
> As for this patch:
>
> My initial reaction was that the new table uses up a good portion of the
> page real-estate (which is okay), but I'm wondering if the table should
> only be displayed if it has values to display. Perhaps someone else could
> share their opinions as well.
>
> There are empty spaces/lines at the following locations:
> - defaultPages.xml @ line 597
> - JVMInformationPage.java @ lines 122, 162, 187, 227, 231, 258, 262, 295,
> 296, 303, 305, 342, 346
>
> The build & uitests pass [0], however because a new component was added to
> the UI I'd like to see a uitest to ensure that everything is working as
> intended.
>
> [0] https://travis-ci.org/aptmac/jmc-qa/builds/548262559
>
>
> >
> > Thank you!
> >
> > Jessye
> >
> > On Wed, Jun 19, 2019 at 3:45 PM Alex Macdonald <almacdon at redhat.com>
> > wrote:
> >
> >> Hi Jessye,
> >>
> >> On Fri, Jun 7, 2019 at 2:48 PM Jessye Coleman Shapiro <
> >> jescolem at redhat.com> wrote:
> >>
> >>> Hi,
> >>>
> >>> This patch addresses JMC-5499: Add XFlagChanged events to JVM
> Information
> >>> or subpage [0].  I have added a JVM Flags Log table to the JVM
> Internals
> >>> Page that displays XFlagChanged events.
> >>>
> >>> Please see the attached patch and let me know what you think.
> >>>
> >>
> >> The content of this patch look good to me.
> >>
> >> However, there are test failures:
> >>
> >> > TestRulesWithJfr.verifyAllResults:132->verifyRuleResults:159
> >>
> >> because the generated JfrRuleBaseline.xml doesn't match the hardcopy one
> >> used to verify the test. This will need to be updated.
> >>
> >> There's also some empty spaces that can be cleaned up.
> >>
> >> JavaBlockingRule.java
> >> - empty space @ lines 134, 138, 151
> >> - trailing empty space @ line 142
> >>
> >>
> >>>
> >>> Thank you!
> >>>
> >>> Jessye Coleman-Shapiro
> >>>
> >>>  [0] https://bugs.openjdk.java.net/browse/JMC-5499
> >>
> >>
> >> Cheers,
> >>
> >> Alex
> >>
> >
>