Re: RFR: 8316372: Monkey Tester Application Part 3 [v2]
On Mon, 25 Mar 2024 07:13:18 GMT, Karthik P K wrote: > I can see the MT app process getting created but no window is showing up. Oh I see - it stored the user choice. Delete the folder `~/.MonkeyTester` (or `\UsersMonkeyTester` and all should be well. - PR Comment: https://git.openjdk.org/jfx/pull/1406#issuecomment-2018173557
Re: RFR: 8316372: Monkey Tester Application Part 3 [v2]
On Fri, 22 Mar 2024 18:23:41 GMT, Andy Goryachev wrote: > > * In all the pages, under Region option, if we select MAX_VALUE for Min > > Height or Min Width, the application hangs or whole window becomes white. I > > observed this issue if we select MIN_VALUE or POSITIVE_INFINITY as well. > > works as designed, doesn't it? it's unlikely that an app developer would set > this value, but a double property does accept this value, so MT allows us to > test the behavior. > Yes the functionality is correct but I'm not sure how it affects MT application. I tried to set MAX_VALUE few times for width and height in different pages and now the MT window is not launching itself. I can see the MT app process getting created but no window is showing up. I'm not sure if there will be any side effects similar to this if someone tries to test it. I'm running MT on Mac 14.3 in IntelliJ IDEA CE. - PR Comment: https://git.openjdk.org/jfx/pull/1406#issuecomment-2017362160
Re: RFR: 8316372: Monkey Tester Application Part 3 [v2]
On Fri, 22 Mar 2024 13:00:02 GMT, Karthik P K wrote:
> * In all the pages, under Region option, if we select MAX_VALUE for Min
> Height or Min Width, the application hangs or whole window becomes white. I
> observed this issue if we select MIN_VALUE or POSITIVE_INFINITY as well.
works as designed, doesn't it?
it's unlikely that an app developer would set this value, but a double property
does accept this value, so MT allows us to test the behavior.
> * In Bubble Chart, when multiple series are present, clear option under
> XYChart clears all but one series.
this button clears the points in the first series. I've added tooltips that
explain the function.
> * In colour picker, if editable option is selected, should we be able to
> change the colour value manually? If yes, I'm not able to edit the colour
> value.
the `ditable` property affects text fields in the "Custom Color..." dialog, and
it works correctly. It probably makes little sense to have the editable
property for this control.
Thanks for pointing this out - even though I did notice that when implementing
the page, but did not investigate.
> tests/manual/monkey/src/com/oracle/tools/fx/monkey/options/FontOption.java
> line 2:
>
>> 1: /*
>> 2: * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights
>> reserved.
>
> Minor: 2023 can be removed
The code also lives in a separate repository where it's copyrighted correctly
(since I constantly add things to it), so no change.
https://github.com/andy-goryachev-oracle/MonkeyTest
> tests/manual/monkey/src/com/oracle/tools/fx/monkey/pages/DemoPage.java line 2:
>
>> 1: /*
>> 2: * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
>
> Minor: Add 2024 here?
thanks for noticing (here and elsewhere)!
> tests/manual/monkey/src/com/oracle/tools/fx/monkey/pages/TreeTableViewPage.java
> line 400:
>
>> 398: s.addChoice("AUTO_RESIZE_NEXT_COLUMN",
>> TreeTableView.CONSTRAINED_RESIZE_POLICY_NEXT_COLUMN);
>> 399: s.addChoice("AUTO_RESIZE_SUBSEQUENT_COLUMNS",
>> TreeTableView.CONSTRAINED_RESIZE_POLICY_SUBSEQUENT_COLUMNS);
>> 400: //s.addChoice("CONSTRAINED_RESIZE_POLICY",
>> TreeTableView.CONSTRAINED_RESIZE_POLICY);
>
> Is there any particular reason why this line is commented?
old code, CONSTRAINED_RESIZE_POLICY is an alias for
CONSTRAINED_RESIZE_POLICY_FLEX_LAST_COLUMN. will clean up.
-
PR Comment: https://git.openjdk.org/jfx/pull/1406#issuecomment-2015670932
PR Review Comment: https://git.openjdk.org/jfx/pull/1406#discussion_r1536018201
PR Review Comment: https://git.openjdk.org/jfx/pull/1406#discussion_r1536018745
PR Review Comment: https://git.openjdk.org/jfx/pull/1406#discussion_r1536020410
Re: RFR: 8316372: Monkey Tester Application Part 3 [v2]
On Wed, 20 Mar 2024 23:56:50 GMT, Andy Goryachev wrote:
>> Further changes to the MonkeyTester application:
>>
>> - remember split pane divider ✔
>> - use 'private' instead of 'protected' in many cases ✔
>> - added more scripts to the 'writing systems' text sample ✔
>> - added RTL window control menu ✔
>> - added embedded swing/fx in tools ✔
>> - added copy popup menu in clipboard viewer ✔
>> - added the custom css field to the css playground tool ✔
>> - added many new pages ✔
>> - split XYChartPage into separate pages ✔
>> - switched to use property sheets (some choices might be incomplete) ✔
>>
>> https://github.com/andy-goryachev-oracle/jfx/blob/8316372.monkey/tests/manual/monkey/README.md
>
> Andy Goryachev has updated the pull request incrementally with one additional
> commit since the last revision:
>
> review comments
Thanks @andy-goryachev-oracle for adding all these options to MonkeyTester.
This will definitely help in testing many aspects of controls easily.
I sanity tested few pages. My observations are listed below.
- In all the pages, under Region option, if we select MAX_VALUE for Min Height
or Min Width, the application hangs or whole window becomes white. I observed
this issue if we select MIN_VALUE or POSITIVE_INFINITY as well.
- In Bubble Chart, when multiple series are present, clear option under XYChart
clears all but one series.
- In colour picker, if editable option is selected, should we be able to change
the colour value manually? If yes, I'm not able to edit the colour value.
Added few inline comments as well.
I will complete the review soon and update here if I have anymore comments.
tests/manual/monkey/src/com/oracle/tools/fx/monkey/options/FontOption.java line
2:
> 1: /*
> 2: * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights
> reserved.
Minor: 2023 can be removed
tests/manual/monkey/src/com/oracle/tools/fx/monkey/pages/DemoPage.java line 2:
> 1: /*
> 2: * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
Minor: Add 2024 here?
tests/manual/monkey/src/com/oracle/tools/fx/monkey/pages/HTMLEditor_Page.java
line 2:
> 1: /*
> 2: * Copyright (c) 2022, 2024, Oracle and/or its affiliates. All rights
> reserved.
Minor: 2022 is not required here right?
tests/manual/monkey/src/com/oracle/tools/fx/monkey/pages/TableViewPage.java
line 385:
> 383: s.addChoiceSupplier("20 Equal", () -> {
> 384: var cs = columnBuilder();
> 385: for(int i=1; i<20; i++) {
Minor: add space after `=` and `<`.
Same comment for ln.no.392
tests/manual/monkey/src/com/oracle/tools/fx/monkey/pages/TreeTableViewPage.java
line 142:
> 140: TreeTableColumn c = newColumn();
> 141: c.setText("C" + System.currentTimeMillis());
> 142: //c.setCellValueFactory((f) -> new
> SimpleStringProperty(describe(c)));
Minor: Commented line can be removed?
tests/manual/monkey/src/com/oracle/tools/fx/monkey/pages/TreeTableViewPage.java
line 400:
> 398: s.addChoice("AUTO_RESIZE_NEXT_COLUMN",
> TreeTableView.CONSTRAINED_RESIZE_POLICY_NEXT_COLUMN);
> 399: s.addChoice("AUTO_RESIZE_SUBSEQUENT_COLUMNS",
> TreeTableView.CONSTRAINED_RESIZE_POLICY_SUBSEQUENT_COLUMNS);
> 400: //s.addChoice("CONSTRAINED_RESIZE_POLICY",
> TreeTableView.CONSTRAINED_RESIZE_POLICY);
Is there any particular reason why this line is commented?
tests/manual/monkey/src/com/oracle/tools/fx/monkey/pages/XYChartPageBase.java
line 2:
> 1: /*
> 2: * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights
> reserved.
Minor: Remove 2023?
tests/manual/monkey/src/com/oracle/tools/fx/monkey/settings/ISettingsProvider.java
line 2:
> 1: /*
> 2: * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
Minor: Any particular reason why only 2022 is retained here?
tests/manual/monkey/src/com/oracle/tools/fx/monkey/tools/EmbeddedJTextAreaWindow.java
line 2:
> 1: /*
> 2: * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
Minor: Change 2023 -> 2024
tests/manual/monkey/src/com/oracle/tools/fx/monkey/util/ShowCharacterRuns.java
line 73:
> 71: HitInfo hit = owner.hitTest(new Point2D(x, y));
> 72: Path p = new Path(owner.rangeShape(hit.getCharIndex(),
> hit.getCharIndex() + 1));
> 73: //System.err.println(i + " " + cs); // FIX
Do we still need this commented line?
tests/manual/monkey/src/com/oracle/tools/fx/monkey/util/Utils.java line 44:
> 42:
> 43: public static void fromPairs(Object[] pairs,
> BiConsumer client) {
> 44: for(int i=0; ihttps://git.openjdk.org/jfx/pull/1406#pullrequestreview-1951849501
PR Review Comment: https://git.openjdk.org/jfx/pull/1406#discussion_r1533681182
PR Review Comment: https://git.openjdk.org/jfx/pull/1406#discussion_r1535164015
PR Review Comment: https://git.openjdk.org/jfx/pull/1406#discussion_r1535166734
PR Review Comment:
Re: RFR: 8316372: Monkey Tester Application Part 3 [v2]
On Wed, 20 Mar 2024 23:56:50 GMT, Andy Goryachev wrote: >> Further changes to the MonkeyTester application: >> >> - remember split pane divider ✔ >> - use 'private' instead of 'protected' in many cases ✔ >> - added more scripts to the 'writing systems' text sample ✔ >> - added RTL window control menu ✔ >> - added embedded swing/fx in tools ✔ >> - added copy popup menu in clipboard viewer ✔ >> - added the custom css field to the css playground tool ✔ >> - added many new pages ✔ >> - split XYChartPage into separate pages ✔ >> - switched to use property sheets (some choices might be incomplete) ✔ >> >> https://github.com/andy-goryachev-oracle/jfx/blob/8316372.monkey/tests/manual/monkey/README.md > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last revision: > > review comments @karthikpandelu Can you also review this? - PR Comment: https://git.openjdk.org/jfx/pull/1406#issuecomment-2013532396
Re: RFR: 8316372: Monkey Tester Application Part 3 [v2]
> Further changes to the MonkeyTester application: > > - remember split pane divider ✔ > - use 'private' instead of 'protected' in many cases ✔ > - added more scripts to the 'writing systems' text sample ✔ > - added RTL window control menu ✔ > - added embedded swing/fx in tools ✔ > - added copy popup menu in clipboard viewer ✔ > - added the custom css field to the css playground tool ✔ > - added many new pages ✔ > - split XYChartPage into separate pages ✔ > - switched to use property sheets (some choices might be incomplete) ✔ > > https://github.com/andy-goryachev-oracle/jfx/blob/8316372.monkey/tests/manual/monkey/README.md Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision: review comments - Changes: - all: https://git.openjdk.org/jfx/pull/1406/files - new: https://git.openjdk.org/jfx/pull/1406/files/b6db646a..2c23ca57 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1406=01 - incr: https://webrevs.openjdk.org/?repo=jfx=1406=00-01 Stats: 4 lines in 3 files changed: 1 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jfx/pull/1406.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1406/head:pull/1406 PR: https://git.openjdk.org/jfx/pull/1406
