On Sun, 15 Sep 2024 12:50:25 GMT, Marius Hanl <mh...@openjdk.org> wrote:
>> Jay Bhaskar has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> copyright year updated along with minor spcae related review
>
> 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 = "";
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
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1760400171
