On Tue, 17 Sep 2024 11:22:56 GMT, Ambarish Rapte <[email protected]> wrote:
>> 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";>
It is ok to use instance variable for SubresourceIntegrityTest. there is no
issue.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1763134034
