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

[Jie Kang]

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