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