On Mon, 16 Sep 2024 12:01:41 GMT, Marius Hanl <mh...@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
