On Wed, 8 Feb 2023 08:14:33 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> The non-hotspot tests integrated with JEP 425/428 were mostly TestNG tests. >> We'd like to convert these JUnit in the main line in advance of other >> updates to these tests in 21. The changes are mostly mechanical and trivial: >> >> - BeforeClass/AfterClass changed to static BeforeAll/AfterAll methods >> - Tests using data providers are changed to parameterized tests >> - The order of the parameters to assertEquals are swapped so that the >> expected result is the first parameter >> - Usages of expectThrows are changed to assertThrows >> - Tests that threw SkipException are changed to the Assumptions API >> >> There are a small number of drive-by changes to the tests, nothing >> significant, e.g. >> >> - GetStackTrace and ParkWithFixedThreadPool changed from "@run testng" to >> "@run main" as they aren't TestNG tests. >> - A few of the tests in StructuredTaskScopeTest for joinXXX are changed to >> use a CountDownLatch rather than sleeping, as the original tests weren't >> very robust. > > Alan Bateman has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains five commits: > > - Merge > - Merge > - Fix typos in comments > - GetStackTrace.java test missing @requires vm.continuations > - Initial commit test/jdk/java/lang/Thread/virtual/GetStackTrace.java line 31: > 29: * @enablePreview > 30: * @modules java.base/java.lang:+open > 31: * @run main GetStackTrace That's interesting - so this test was marked as a testng test previously, but had not test methods in it. I had a look at the test run history of this one and I see that the test execution results in: > java/lang/Thread/virtual/GetStackTrace.java > Total tests run: 0, Passes: 0, Failures: 0, Skips: 0 I wonder if testng (and junit) can be configured to fail the test if no test methods were present, to flag such mistakes. Keeping that aside, the change to make this `@run main` looks fine. As for adding the `@requires vm.continuations`, I'm not familiar with its expectations. Looking at the test case, it just captures stacktraces using `Thread.getStackTrace()` and then just does some basic validation of those stacktrace elements. Do we use this `@requires vm.continuations` even in such tests? ------------- PR: https://git.openjdk.org/jdk/pull/12426