On Mon, 20 Apr 2026 03:54:48 GMT, SendaoYan <[email protected]> wrote:

>> Daniel Fuchs has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Review feedback
>
> test/jdk/java/util/logging/LogRecordThreadIdTest.java line 46:
> 
>> 44:     private static LogRecord record, record1, record2;
>> 45: 
>> 46:     @BeforeAll
> 
> LogRecordThreadIdTest previously used TestNG @BeforeTest, which runs before 
> each @Test method, so each test started with freshly constructed LogRecord 
> instances. With JUnit @BeforeAll + static fields, the same instances are 
> reused. The assertions still look safe because each test overwrites the state 
> it cares about, but could you confirm this was intentional? If you want to 
> preserve the old “fresh records per test” behavior, @BeforeEach (possibly 
> with non-static fields) would be closer to TestNG.

As far as I understand `@BeforeTest` means that the method will be run before 
any test in the test group. If there is no test group, the test group is the 
class. Which means that BeforeAll/BeforeTest/BeforeClass are all equivalent 
here. That said there's no real reason to use a setup method here. I simply 
removed it.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/30769#discussion_r3148095343

Reply via email to