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.
@Maran23 is right. In the absence of support for parameterized class-level
tests the easiest solution (I think) is to parameterize every `@Test`, which
means:
- add parameters to each method, replace `@Test` with `@ParameterizedTest` and
`@MethodSource`
- add parameters to setup method (`@Before`, remove the annotation)
- call the setup method from each test method
- remove the fields that were set in the constructor and the constructor itself
Things might be a bit more complicated in the case of test class hierarchies,
so additional work and checks will be needed.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1567#discussion_r1761320750
