On Mon, 16 Sep 2024 23:03:50 GMT, Jay Bhaskar <jbhas...@openjdk.org> wrote:
>> Successfully converted web tests from JUnit 4 to JUnit 5, ensuring all tests
>> are fully compliant with the JUnit 5 framework.
>
> Jay Bhaskar has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Year change should be reverted as main
Providing a few comments, majority are related to correcting indentation
correction.
1. Only space changes: Some indentation changes only add or remove spaces. I
would suggest to avoid these changes. Or add only if the existing formatting is
very bad. But would recommend to avoid.
2. Some indendations are bad, which needs correction for example having `);` on
a separate line.
Another comment is regarding Parameterized test:
modules/javafx.web/src/test/java/test/javafx/scene/web/SubresourceIntegrityTest.java
Looks like the test is not converted correctly. The parameters are never
assigned to the member variables. Hence never tested. Please take a look at the
specific comment on the test.
modules/javafx.web/src/test/java/test/javafx/scene/web/BindingTest.java line 86:
> 84: 0.0,
> 85: "WebPage zoom factor"
> 86: );
Minor reformatting could be done: `0.0, "WebPage zoom factor");`
modules/javafx.web/src/test/java/test/javafx/scene/web/CSSTest.java line 368:
> 366: " <button class=\"bad3\">D</button>\n" +
> 367: " </body>\n" +
> 368: "</html>"
The formatting change can be reverted.
modules/javafx.web/src/test/java/test/javafx/scene/web/CallbackTest.java line
200:
> 198: clear();
> 199: script = JS_PROMPT.replaceAll("MESSAGE", message)
> 200: .replaceAll("DEFAULT", defaultValue);
only formatting change, could be reverted.
modules/javafx.web/src/test/java/test/javafx/scene/web/CallbackTest.java line
282:
> 280: Rectangle2D r = ev.getData();
> 281: called(RESIZED, r.getMinX(), r.getMinY(),
> 282: r.getWidth(), r.getHeight());
only formatting change, could be reverted.
modules/javafx.web/src/test/java/test/javafx/scene/web/CanvasTest.java line 50:
> 48: // JDK-8162922
> 49: @Test public void testCanvasStrokeRect() {
> 50: final String htmlCanvasContent = "\n"
Only formatting change, should be reverted.
modules/javafx.web/src/test/java/test/javafx/scene/web/CanvasTest.java line 81:
> 79: 255, 0, 0, 255,
> 80: 255, 0, 0, 255,};
> 81: */
only formatting change, could be reverted.
modules/javafx.web/src/test/java/test/javafx/scene/web/CanvasTest.java line 91:
> 89: 0, 0, 0, 0,
> 90: 0, 0, 0, 0,
> 91: 0, 0, 0, 0};
only formatting change, could be reverted.
modules/javafx.web/src/test/java/test/javafx/scene/web/CanvasTest.java line 140:
> 138: + "ctx.fillStyle = pattern;\n"
> 139: + "ctx.fillRect(0, 0, 100, 100);\n"
> 140: + "</script>\n";
only formatting change, could be reverted.
modules/javafx.web/src/test/java/test/javafx/scene/web/CanvasTest.java line 167:
> 165: ),
> 166: "First rect center"
> 167: );
formatting/ indentation needs correction. for example:
assertEquals(0,
(int)
getEngine().executeScript("document.getElementById('canvaspattern').getContext('2d').getImageData(1,
1, 1, 1).data[0]"),
"Pattern top-left corner");
modules/javafx.web/src/test/java/test/javafx/scene/web/CanvasTest.java line 186:
> 184: + "</script>"
> 185: + "</body>"
> 186: , mime);
only formatting change, could be reverted.
modules/javafx.web/src/test/java/test/javafx/scene/web/CanvasTest.java line 196:
> 194: exMessage.contains("Exception") ||
> exMessage.contains("Error"),
> 195: String.format("Test failed with exception:\n%s",
> exMessage)
> 196: );
Indentation correction:
assertFalse(exMessage.contains("Exception") || exMessage.contains("Error"),
String.format("Test failed with exception:\n%s", exMessage));
modules/javafx.web/src/test/java/test/javafx/scene/web/WebViewTest.java line
107:
> 105: " </div>" +
> 106: "</body> </html>"
> 107: );
Indentation change, could be reverted.
-------------
Changes requested by arapte (Reviewer).
PR Review: https://git.openjdk.org/jfx/pull/1567#pullrequestreview-2309330479
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1762989324
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1762993041
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1762995926
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1762995750
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1762996408
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1762997205
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1762997327
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1762998971
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1763004526
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1762999828
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1763009212
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1763019008
