JDK-8208070 : [MacOS] Write a manual test for fix done for JDK-8191885
Type:Bug
Component:javafx
Sub-Component:window-toolkit
Affected Version:openjfx11
Priority:P3
Status:Resolved
Resolution:Fixed
OS:os_x
CPU:generic
Submitted:2018-07-23
Updated:2018-12-18
Resolved:2018-12-18
The Version table provides details related to the release that this issue/RFE will be addressed.
Unresolved : Release in which this issue/RFE will be addressed. Resolved: Release in which this issue/RFE has been resolved. Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.
The fix done for JDK-8191885 can't be verified by an automatic test. So a manual test needs to be written.
Comments
Changeset: 9fb20df0a6fc
Author: pbansal
Date: 2018-12-18 23:58 +0530
URL: http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/9fb20df0a6fc
8208070: [MacOS] Write a manual test for fix done for JDK-8191885
Reviewed-by: kcr, mbilla
18-12-2018
+1
18-12-2018
As mentioned offline, this doesn't need a second reviewer. Murali can push the fix for you when ready.
18-12-2018
looks fine.
Minor correction: please correct the word "MacOs" to either MacOS or MacOSX. (no need for separate webrev).
11-12-2018
[~mbilla] Thanks for the review.
webrev: http://cr.openjdk.java.net/~pbansal/8208070/webrev.02/
11-12-2018
I missed this in my earlier review.
1. You can simplify the below code : (you don't need braces for single parameter and single line lambda body.
passButton.setOnAction((e)->{
quit();
});
change to below:
passButton.setOnAction(e -> quit());
2. Also for below code , you can remove braces around "e"
failButton.setOnAction((e)->{
quit();
throw new AssertionError("The window is no longer resizable");
});
Change to:
failButton.setOnAction(e -> {
quit();
throw new AssertionError("The window is no longer resizable");
});
Please give a space between "e" and "->" . I don't see this correction in webrev.01
11-12-2018
Few minor comments:
1. Please add a space for below statements:
public class WindowResizableTest extends Application{ -> add a space between Application and {
passButton.setOnAction((e)->{ -> add a space between "(e) and ->" and "-> and {"
like: passButton.setOnAction( (e) -> {
similarly for failButton.setOnAction((e)->{
2. Please rephrase below sentence
new Label("1. This is a MacOs specific test. If you are on some other Platform, please click Pass."),
To
new Label("1. This is a MacOS specific test. If you run the test on other platforms, please click Pass."),
3. Typo correction for "begining"
new Label("2. Verify that the window is resizable in begining by dragging the edges of this window."),
4. rephrase the below sentences:
new Label("3. Click the little green maximize button on window to enter full-screen mode and again press it to come back to non-fullscreen mode."),
To
new Label("3. Click the maximize button(in green) on window to enter full screen mode and again click it to come back to normal mode."),
new Label("4. If the window is still resizable, Press Pass otherwise Fail."),
To
new Label("4. If the window is still resizable, click Pass otherwise Fail."),
5. public static void quit() method can be private?
6. Please add a space in between "," and failButton
new HBox(10, passButton,failButton));
7. You are throwing exception after Platform.exit(). Are you able to see the exception in fail case?
throw new AssertionError("The window is no longer resizable");
AFAI, Platform.exit will call stop() method (if you override) and you can keep the clean up code in stop() after calling Platform.exit.
8. You can remove extra empty lines before the statement VBox rootNode = new VBox(5,
11-12-2018
[~mbilla] thanks for the review. Below is the updated webrev. I am able to get the exception if I press the fail button. So it is working as expected.
webrev: http://cr.openjdk.java.net/~pbansal/8208070/webrev.01/
11-12-2018
[~kcr][~mbilla]
Please review the manual test.
webrev: http://cr.openjdk.java.net/~pbansal/8208070/webrev.00/
11-12-2018
Targeted to openjfx12, but if this can be done in time it can go into 11.