On Sun, 15 Sep 2024 12:31:03 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.
There is one major issue with the parameterized test as far as I can see. Some
minor style guide things, looks good otherwise!
modules/javafx.web/src/test/java/test/com/sun/webkit/network/CookieTest.java
line 454:
> 452: }
> 453:
> 454: assertEquals(expected.name, actual.getName(),"Unexpected name" +
> s);
minor: missing space after the comma -> `, `
modules/javafx.web/src/test/java/test/com/sun/webkit/network/CookieTest.java
line 455:
> 453:
> 454: assertEquals(expected.name, actual.getName(),"Unexpected name" +
> s);
> 455: assertEquals(expected.value, actual.getValue(),"Unexpected
> value" + s);
minor: missing space after the comma -> `, `
modules/javafx.web/src/test/java/test/javafx/scene/web/FileReaderTest.java line
248:
> 246: assertEquals(expectedLength, actualLength, message);
> 247: for (int i = 0; i < expectedArrayBuffer.length; i++) {
> 248: assertEquals(expectedArrayBuffer[i],
> ((Number)(obj.getSlot(i))).byteValue(),"Unexpected file content received");
minor: missing space after the comma -> `, `
modules/javafx.web/src/test/java/test/javafx/scene/web/LoadTest.java line 132:
> 130: Element html = doc.getDocumentElement();
> 131: assertNotNull(html, "There should be an HTML element");
> 132: assertEquals("HTML", html.getTagName(),"HTML element should
> have tag HTML");
minor: missing space after the comma -> `, `
modules/javafx.web/src/test/java/test/javafx/scene/web/LoadTest.java line 218:
> 216: WebEngine webEngine = new WebEngine();
> 217: webEngine.titleProperty().addListener((observable, oldValue,
> newValue) -> {
> 218: assertTrue(webEngine.getLoadWorker().getState() ==
> SUCCEEDED,"loadContent in SUCCEEDED State");
minor: missing space after the comma -> `, `
modules/javafx.web/src/test/java/test/javafx/scene/web/SubresourceIntegrityTest.java
line 99:
> 97: @ParameterizedTest
> 98: @MethodSource("dataProvider")
> 99: public void testScriptTagWithCorrectHashValue() {
Major: This methods need the parameters from the `MethodSource` AFAIK, that is:
`testScriptTagWithCorrectHashValue(String hashValue, String expected) { .. }`
The member variables should be removed then:
```
private final String hashValue = "";
private final String expected = "";
-------------
Changes requested by mhanl (Committer).
PR Review: https://git.openjdk.org/jfx/pull/1567#pullrequestreview-2305353159
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1760039063
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1760039105
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1760039605
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1760040160
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1760040230
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1760041171
