RFR: 8339515: [TestBug] Convert web tests to JUnit 5 [v8]
Ambarish Rapte
arapte at openjdk.org
Tue Sep 17 11:36:17 UTC 2024
On Mon, 16 Sep 2024 12:01:41 GMT, Marius Hanl <mhanl at openjdk.org> wrote:
>> In this case, keeping the values as instance variables is more reliable. Since all the tests pass when hashValue and expected are instance variables and some of them fail when I adding parameter to testScriptTagWithCorrectHashValue ,
>>
>> it's likely that their assignment happens at the correct time relative to the @BeforeEach setup. Using parameters may interfere with the test lifecycle, since hasValue is being used in
>> @BeforeEach
>> public void setup() ....{}
>>
>> it's safer and more straightforward to use instance variables.This would also make it easier to manage the state across @BeforeEach
>
> I think you need to do the same as done by @andy-goryachev-oracle in his JUnit5 replacement PR:
>
> public void setup(String hashValue, String expected) throws Exception {
> htmlFile = new File("subresource-integrity-test.html");
> ...
> }
>
> @ParameterizedTest
> @MethodSource("dataProvider")
> public void testScriptTagWithCorrectHashValue(String hashValue, String expected) {
> setup(hashValue, expected);
> ...
> }
>
>
> This way you dont need the instance fields, as they will be empty all the time and never assigned.
I also think the test needs to be modified as suggested by @Maran23 and @andy-goryachev-oracle .
With the changes in this PR the member variables are never assigned with value.
I verified the values of the two member variables by printing them before line 102, They are always empty.
Apply below change, after applying this PR changes.
diff --git a/modules/javafx.web/src/test/java/test/javafx/scene/web/SubresourceIntegrityTest.java b/modules/javafx.web/src/test/java/test/javafx/scene/web/SubresourceIntegrityTest.java
index e65ff7d32f..0d2d670777 100644
--- a/modules/javafx.web/src/test/java/test/javafx/scene/web/SubresourceIntegrityTest.java
+++ b/modules/javafx.web/src/test/java/test/javafx/scene/web/SubresourceIntegrityTest.java
@@ -99,6 +99,8 @@ public final class SubresourceIntegrityTest extends TestBase {
load(htmlFile);
final String bodyText = (String) executeScript("document.body.innerText");
assertNotNull("document.body.innerText must be non null for " + hashValue, bodyText);
+ System.err.println("hashValue = " + hashValue);
+ System.err.println("expected = " + expected);
assertEquals(hashValue, expected, bodyText);
}
<img width="638" alt="Screenshot 2024-09-17 at 4 51 46 PM" src="https://github.com/user-attachments/assets/5dd4360f-a82d-46b6-8828-3406d63a214f">
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1763063040
More information about the openjfx-dev
mailing list