RFR: JMC-5473: Quick search for Automated Analysis Result Table

[Jie Kang]

Hi all,

Carmine has requested I continue this work in his stead. Please find
an updated webrev for review below.

Webrev: http://cr.openjdk.java.net/~jkang/jmc-5473/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JMC-5473


Regards



On Thu, Aug 1, 2019 at 10:24 AM Carmine Vincenzo Russo
<carusso at redhat.com> wrote:
>
> Hi Jie,
>
> Thanks for pointing out these errors, I'll fix them and get back with an updated version.
>
> Thanks,
> Carmine
>
> On Thu, Aug 1, 2019 at 4:16 PM Jie Kang <jkang at redhat.com> wrote:
>>
>> On Thu, Aug 1, 2019 at 6:53 AM Carmine Vincenzo Russo
>> <carusso at redhat.com> wrote:
>> >
>> > Hi,
>> >
>> > The attached patch addresses the issue JMC-5473[0] in which a search
>> > feature for the Automated Analysis Result Table was lacking.
>> >
>> > In ResultOverview.java, I added the text component and a simple layout to
>> > give the page more consistency when the table is displayed.
>> > I also produced ResultOverviewTest.java to test if the table has
>> > components, the search feature operates, and two more to check if the
>> > behaviour is what we expect with a nonsense search and a specific search.
>> >
>> > A side note, since there were no tests for the Automated Analysis Result, I
>> > had to add the tab in JfrUi and allow the record analysis in two other
>> > classes, OldRecordingsVerificationTest.java and JfrRecordingTest.java,
>> > because they go through all the tabs listed in JfrUi during their tests.
>> >
>> > How does it look?
>>
>> Hi Carmine,
>>
>> Thanks for doing this; the tests are appreciated! I have some issues
>> to address below:
>>
>> diff --git a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/overview/R
>> esultOverview.java
>> b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/ov
>> erview/ResultOverview.java
>> --- a/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/overview/ResultOv
>> erview.java
>> +++ b/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/overview/ResultOv
>> erview.java
>> [...]
>> -
>> +
>>
>> * Whitespace additions like the example shown above should be removed;
>> there are others in the patch.
>>
>> -                       Map<Result, DataPageDescriptor> map = createResultMap();
>> +                       GridLayout layout = new GridLayout(1, true);
>> +                       this.form.getBody().setLayout(layout);
>> +                       text = new Text(form.getBody(), SWT.BORDER |
>> SWT.HORIZONTAL | SWT.SEARCH | SWT.RESIZE);
>> +                       text.setLayoutData(new GridData(SWT.FILL,
>> SWT.DEFAULT, true, false));
>> +                       text.setMessage(Messages.ResultOverview_SEARCH_TABLE);
>> +                       text.setToolTipText(Messages.ResultOverview_SEARCH_BAR);
>> +                       Map<Result, DataPageDescriptor> map =
>> createResultMap(text.getText());
>>                         table = new ResultTableUi(form, toolkit,
>> editor, loadedState, map);
>> +                       ModifyListener listener = new ModifyListener() {
>> +                               @Override
>> +                               public void modifyText(ModifyEvent e) {
>> +                                       Map<Result,
>> DataPageDescriptor> map = createResultMap(text.getText());
>> +                                       table.updateInput(map);
>> +                               }
>> +                       };
>> +                       text.addModifyListener(listener);
>>                 }
>> * The additions to the Table UI done above should occur in the Table
>> UI code, rather than in the overview which is managing the HTML UI and
>> the Table UI, ie. the above should be within ResultTableUi.java
>>
>> * When switching to the Table and then back to the HTML, the resulting
>> view is strange; see the linked image [1]. This may have to do with
>> how the form's layout is modified by your addition. This form is the
>> parent for the ResultTableUi. I would add children to it with custom
>> layouts, but I would not modify the 'parent' layout itself.
>>
>> https://imgur.com/a/nS1m2T2
>>
>>
>> Regards,
>>
>> >
>> > Regards,
>> > Carmine
>> >
>> > [0] https://bugs.openjdk.java.net/browse/JMC-5473
>> >
>> > --
>> > Carmine Vincenzo Russo
>
>
>
> --
> Carmine Vincenzo Russo