8382264: Refactor java.logging TestNG tests to use JUnit#30769
8382264: Refactor java.logging TestNG tests to use JUnit#30769dfuch wants to merge 4 commits intoopenjdk:masterfrom
Conversation
|
👋 Welcome back dfuchs! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
Webrevs
|
| * @run testng GetResourceBundleTest | ||
| * @run junit GetResourceBundleTest |
There was a problem hiding this comment.
Does this intentionally not use ${test.main.class}? (which you changed in the other test classes)
|
|
||
| @BeforeTest | ||
| public void setUp() throws Exception { | ||
| @BeforeAll |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Please find here a small change that converts
java.loggingTestNG tests to use JUnit. Most java.logging tests are plain jtreg tests, so this is a small set.Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/30769/head:pull/30769$ git checkout pull/30769Update a local copy of the PR:
$ git checkout pull/30769$ git pull https://git.openjdk.org/jdk.git pull/30769/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 30769View PR using the GUI difftool:
$ git pr show -t 30769Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/30769.diff
Using Webrev
Link to Webrev Comment